-
Notifications
You must be signed in to change notification settings - Fork 20
impl: enable log and mw_log
#133
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
License Check Results🚀 The license check job ran with the Bazel command: bazel run //:license-checkStatus: Click to expand output |
|
The created documentation from the pull request is available at: docu-html |
d52bb07 to
6a71358
Compare
6a71358 to
dbe1d06
Compare
dbe1d06 to
46fd548
Compare
| // Load KVS file and parse from string to `JsonValue`. | ||
| let json_str = fs::read_to_string(kvs_path)?; | ||
| let json_value = Self::parse(&json_str)?; | ||
| debug!("Loading KVS file: {kvs_path:?}"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some of the debug messages seem a bit redundant in the case of an error, for example this one you would get :
"Loading KVS file: kvs_path"
"Failed to load KVS file: kvs_path"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case yes, to a degree. Imagine following scenarios though:
- Loading succeeded, but parsing failed. Thanks to those logs You have information about steps being done and state of KVS without having to look at the code.
- Something went extremely wrong during
fs::read_to_stringand app segfaulted. Thanks to that You have last known operation done.
I have the following rule of thumb for log levels:
trace- extremely granular information what changed (e.g., operation result, function-local state changed). Rarely used.debug- information what is being done right now (before it happen).info- global-state change.warn- non-critical pre- or post- condition failed.error- critical pre- or post-condition failed.
| })?; | ||
|
|
||
| // Perform hash check. | ||
| debug!("Performing hash check, KVS file: {kvs_path:?}, hash file: {hash_path:?}"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same thing here, the first one it makes the second message redundant
46fd548 to
f567596
Compare
- Add more logs from the implementation. - Fix tests. - Fix building from Cargo and Bazel. - Disable `score-log` feature in CI check.
f567596 to
d3044d8
Compare
score-logfeature in CI check.