Fix increasing the root logging level by log-capture #987
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jenisys
force-pushed
the
main
branch
5 times, most recently
from
6508bd4
to
a4dd3fe
Compare
jenisys
force-pushed
the
main
branch
2 times, most recently
from
11ddaf9
to
5101465
Compare
janoskut
force-pushed
the
fix-increasing-root-logging-level
branch
from
1d53ec0
to
0a855ab
Compare
jenisys
force-pushed
the
main
branch
3 times, most recently
from
7ec93d2
to
46ad983
Compare
janoskut
force-pushed
the
fix-increasing-root-logging-level
branch
from
0a855ab
to
b2c4f66
Compare
janoskut
force-pushed
the
fix-increasing-root-logging-level
branch
from
b2c4f66
to
57c2189
Compare
fix tag expression
Rationale: In behave, loggers are usually initialized before "before_all()" and "setup_logging()" are invoked. When using a file config, the default behavior of the logging module is to disable existing loggers (see warning in https://docs.python.org/3/howto/logging.html#configuring-logging). Changing the default behavior to not disabling the existing loggers matches behaves logger users better. By passing the **kwargs into "fileConfig()", we still allow all desired behaviors.
janoskut
force-pushed
the
fix-increasing-root-logging-level
branch
from
57c2189
to
7ee1286
Compare
Changing the main argument parser to accept "unknown" args. Let the Runner superclass parse "unknown" args into "extra args". The default Runner implementation doesn't accept any extra args, to restore the original semantics. However, having that, custom Runner classes can now provide own argparsers to accept extra-args from the CLI.
ADD allowing extra CLI args to be passed to Runners
Log-capturing used to set the root logger level to the configured capturing level, so that it is sufficiently low to capture the desired level. However when the root logging level is already lower, this will in fact increase the level, changing the level of all other registered handlers. This change fixes this: Not to directly set the root logging level, but instead only decrease it if necessary.
janoskut
force-pushed
the
fix-increasing-root-logging-level
branch
from
7ee1286
to
d410c86
Compare
|
HINT: |
jenisys
force-pushed
the
main
branch
4 times, most recently
from
fe1ca4d
to
fcfe5af
Compare
jenisys
force-pushed
the
main
branch
2 times, most recently
from
0a4d73b
to
2c11d2e
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Commit Message
Discussion
Honestly, I don't think that the root level should be touched at all in here. For me it seems like that
behaveassumes that there will ever be only one of those 2 handlers: the capturing handler, or the default stream handler.However, as soon as someone (like me) needs to use other handlers, such as file handlers, the root level must not be changed, otherwise those other handlers don't get to see the level they're configured for.
See the
before_all()code in the feature file. This is the only way I came up with to support:Note that with this requirement I also can't use
Configuration.setup_logging(), because this sets up the default stream handler which is tied to the root logging level. This function also manipulates the root level. I'm personally wondering, if this function is actually more harmful (=causing confusion) than that it is helpful. Also it hides the not too minor detail that no console handler is created (bylogging.basicConfig()), when in--logcapturemode - because then, log_capture has already added a handler, andlogging.basicConfig()only creates a handler when none is present yet.Quite much hidden stuff in there... the above change (or removing setting the root logger) seems to be definitely required to allow users to manage the handlers freely.