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

#985 Don't disable existing loggers by default #986

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

janoskut
Copy link

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.

@jenisys
Copy link
Member

jenisys commented Dec 28, 2021

Correction of description above:

  • behave does not initialize the logging subsystem before calling the before_all() hook. If this is the case, it will be corrected.
  • BUT: The user code that is imported via „environment.py“ may pre-initialize the logging subsystem.
  • The logging module/subsystem uses the init-once only strategy which causes must of the problems or misunderstandings when using it.

@janoskut
Copy link
Author

@jenisys this actually exactly what I meant. Does it make it clearer if I change the first sentence to:

In behave, loggers are usually initialized by user code before "before_all()" and "setup_logging()" are invoked.

More rationale in #985.

@jenisys
Copy link
Member

jenisys commented Dec 28, 2021

No (no change needed from my side), I just wanted to clarify the point that it can be understood by others when reading the ticket.

@janoskut
Copy link
Author

janoskut commented Feb 5, 2022

@jenisys is there something I can do to push this change further, like help to clarify any doubts, or add more tests?

@jenisys
Copy link
Member

jenisys commented Feb 5, 2022

I am working on it.

@jenisys jenisys force-pushed the main branch 5 times, most recently from 6508bd4 to a4dd3fe Compare March 6, 2022 23:33
@jenisys jenisys force-pushed the main branch 2 times, most recently from 11ddaf9 to 5101465 Compare May 2, 2022 19:16
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 janoskut force-pushed the prevent-disable-existing-loggers branch from 8f12ad1 to 7dfef97 Compare October 26, 2022 14:50
@jenisys jenisys force-pushed the main branch 4 times, most recently from fe1ca4d to fcfe5af Compare April 22, 2023 17:20
@jenisys jenisys force-pushed the main branch 2 times, most recently from 0a4d73b to 2c11d2e Compare May 14, 2024 22:39
@jenisys jenisys force-pushed the main branch 2 times, most recently from 3e51dda to c6ab01c Compare May 26, 2024 15:00
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

Successfully merging this pull request may close these issues.

None yet

2 participants