search

Eraden / amdgpud

MIT License
195 stars 11 forks source link

Mistakes in config parsing #34

Closed house-of-vanity closed 2 years ago

house-of-vanity commented 2 years ago

Hi, first of all your project is awesome! Wanna report an issue. I've build v1.0.8 tag and found an issue with config parsing. I used default example config from examples and got:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Error { inner: ErrorInner { kind: Custom, line: Some(24), col: 0, at: Some(268), message: "missing field `interval`", key: [] } }', /home/ab/repos/amdgpud/amdgpu/src/utils.rs:92:58 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace Aborted (core dumped)

I've made some research and know the point. Here is a default values but it works if neither interval nor log_level are defined: https://github.com/Eraden/amdgpud/blob/de5f2b77cb9f48f14f86412aa5b3a36c753e8174/amdgpu-config/src/monitor.rs#L22-L29

But if I have config without interval but log_level and vice versa it fails. Looks like need to check unexciting values and define it with default. Here is an error, config.interval doesn't exist: https://github.com/Eraden/amdgpud/blob/de5f2b77cb9f48f14f86412aa5b3a36c753e8174/amdgpu-config/src/monitor.rs#L44-L50 Rust isn't my strong point so I can't fix it in a fancy way.

Eraden commented 2 years ago

TOML for some reason requires simple types first and structures to be last.

I didn't applied this in new config and it paniced. Will be fixed in upcoming 1.0.9

Eraden commented 2 years ago

@house-of-vanity please confirm this was fixed

house-of-vanity commented 2 years ago

Should work. Can't check right now, I escaped from Russia/Ukraine conflict without my PC.

Eraden commented 2 years ago

I'm sorry to hear this. I hope you are safe and in peaceful country now