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

Log level is leaking between invokations: hack to force-reset it #29

Open
kdeldycke opened this issue Sep 20, 2021 · 2 comments
Open

Log level is leaking between invokations: hack to force-reset it #29

kdeldycke opened this issue Sep 20, 2021 · 2 comments

Comments

@kdeldycke
Copy link
Contributor

The log level is set once and for all in the option in the plugin.

That mean in a test suite that is playing with the log-level defaults, the level set within a previous test is carried upon the next. This happened to me in my project where I tested the effect of defaults loaded from the default_map.

I found a way to fix this issue by resetting the log level after each CLI execution.

This fix should be merged upstream to this plugin. One way to properly does it is I guess to leverage the resource management utilities available in Click.

@untitaker
Copy link
Collaborator

this seems feasible. I see one other option, that is to enforce the log level in a log filter, then the log level can live on the click context instead of being global state. click context is a threadlocal too, so this would make it threadsafe

@kdeldycke
Copy link
Contributor Author

Since my initial bug report I revisited this issue. I have a better understanding of Click's architecture. And you're right. the Context was indeed designed to be thread-safe, so that might be a good way to address the issue.

In the mean time I also tried to pack all my workarounds into a standalone option: https://github.com/kdeldycke/click-extra/blob/aff2623194606dde322cf2bed454ae3805c04214/click_extra/logging.py#L46-L85

See how I ended up using a combo of callback and ctx.call_on_close() to set-up and reset the level. This seems to neutralize my hack that requires any meddling with default_map itself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants