Skip to content

Make --debug flag override RUST_LOG#3208

Merged
2 commits merged intothin-edge:mainfrom
Bravo555:fix/flag-override-rust-log
Oct 28, 2024
Merged

Make --debug flag override RUST_LOG#3208
2 commits merged intothin-edge:mainfrom
Bravo555:fix/flag-override-rust-log

Conversation

@Bravo555
Copy link
Copy Markdown
Member

Proposed changes

Flags should always take precedence over environment variables because are often used by users to override the environment which they usually have little control over.

To implement this change it was necessary to refactor the code a little bit as previously set_log_level function was not aware if desired log level was specifically requested by the user (--debug flag) or just the default (INFO level). As such, the new log_init function takes into account all sources that decide what log level we should select: CLI options, RUST_LOG env variable, and tedge configuration file.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Improvement (general improvements like code refactoring that doesn't explicitly fix a bug or add any new functionality)
  • Documentation Update (if none of the other choices apply)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Paste Link to the issue


Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA (in all commits with git commit -s)
  • I ran cargo fmt as mentioned in CODING_GUIDELINES
  • I used cargo clippy as mentioned in CODING_GUIDELINES
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

To be merged before #3205

@Bravo555 Bravo555 added theme:troubleshooting Theme: Troubleshooting and remote control theme:cli Theme: cli related topics labels Oct 28, 2024
@Bravo555 Bravo555 temporarily deployed to Test Pull Request October 28, 2024 11:30 — with GitHub Actions Inactive
@Bravo555 Bravo555 mentioned this pull request Oct 28, 2024
11 tasks
@codecov
Copy link
Copy Markdown

codecov bot commented Oct 28, 2024

Codecov Report

Attention: Patch coverage is 0% with 47 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...mon/tedge_config/src/system_services/log_config.rs 0.00% 27 Missing ⚠️
crates/core/tedge_agent/src/lib.rs 0.00% 5 Missing ⚠️
crates/core/tedge_mapper/src/lib.rs 0.00% 5 Missing ⚠️
crates/core/tedge_watchdog/src/lib.rs 0.00% 5 Missing ⚠️
plugins/c8y_firmware_plugin/src/lib.rs 0.00% 5 Missing ⚠️
Additional details and impacted files

📢 Thoughts on this report? Let us know!

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Oct 28, 2024

Robot Results

✅ Passed ❌ Failed ⏭️ Skipped Total Pass % ⏱️ Duration
523 0 2 523 100 1h32m46.284270999s

Flags should always take precedence over environment variables because
are often used by users to override the environment which they usually
have little control over.

To implement this change it was necessary to refactor the code a little
bit as previously set_log_level function was not aware if desired log
level was specifically requested by the user (--debug flag) or just the
default (INFO level). As such, the new log_init function takes into
account all sources that decide what log level we should select: CLI
options, RUST_LOG env variable, and tedge configuration file.

Signed-off-by: Marcel Guzik <marcel.guzik@inetum.com>
Signed-off-by: Marcel Guzik <marcel.guzik@inetum.com>
@Bravo555 Bravo555 force-pushed the fix/flag-override-rust-log branch from b505007 to 537e02d Compare October 28, 2024 14:53
@Bravo555 Bravo555 temporarily deployed to Test Pull Request October 28, 2024 14:53 — with GitHub Actions Inactive
log_init(
"tedge-mapper",
&mapper_opt.log_args,
&tedge_config_location.tedge_config_root_path,
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously erroneously used &tedge_config_location.tedge_config_file_path instead of root_path here, squashed and rebased a fix.

Copy link
Copy Markdown
Contributor

@didier-wenzek didier-wenzek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved

@github-merge-queue github-merge-queue bot closed this pull request by merging all changes into thin-edge:main in ad03ca3 Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

theme:cli Theme: cli related topics theme:troubleshooting Theme: Troubleshooting and remote control

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants