Skip to content
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

Share log attr between cli classes #759

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

evrose54
Copy link
Contributor

Reviewer Checklist

  • Required existing tests pass (ie full_test.sh, others as appropriate)
  • NO REQUIRED unit tests (explain why not required)
  • NO REQUIRED integration tests (explain why not required)
  • Required documentation added for new/modified functionality
  • Required release notes added for new/modified functionality
  • NO REQUIRED updates to other repos (explain why not required)

Related Issues

fixes #714

Testing Instructions

pytest -v /tests/unit_tests/commandline/.

Summary

Since we've started developing the GeoIPS CLI, we've known that there was a need to
have a shared logger object among all of the CLI command classes for debugging
purposes and to output basic information to the user if requested. We can now do
this using the '--log-level' / '-log' flag. Depending on the log level set, all
levels that match or are of higher 'order' than the log level provided will be
outputted. This functionality matches that used in 'run_procflow'. Logging is now
a supported utility throughout the CLI, and can be used in any CLI class via
self.LOG.<log_level>(log_statement).

Output

I didn't add any permanent logging statements but we can add some if wanted. I did test the new functionality and it works as expected!

@evrose54 evrose54 added refactor Code refactoring updates cli Command line interface tasks labels Aug 26, 2024
@evrose54 evrose54 marked this pull request as draft August 26, 2024 17:32
@evrose54 evrose54 marked this pull request as ready for review August 27, 2024 16:02
type=str,
default="interactive",
choices=["debug", "error", "info", "interactive", "warning"],
help="The logging level to use for output via the CLI.",
choices=["interactive", "debug", "info", "warning", "error"],
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this line should be reverted to the previous order.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems interactive is set to level 35. Debug is 10, info is 20, warning is 30, error is 40, critical is 50. So the list should actually be in the order of [debug, info, warning, interactive, error, critical].

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This has since been addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Command line interface tasks refactor Code refactoring updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Discuss how to share LOG attribute in the CLI
2 participants