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

Components logger doesn't respect default logger level #23819

Closed
zewelor opened this issue May 11, 2019 · 4 comments · Fixed by #26675
Closed

Components logger doesn't respect default logger level #23819

zewelor opened this issue May 11, 2019 · 4 comments · Fixed by #26675

Comments

@zewelor
Copy link
Contributor

zewelor commented May 11, 2019

Home Assistant release with the issue: 0.92.2

Last working Home Assistant release (if known):

**Operating environment (Hass.io/Docker/Windows/etc.):**Docker

Component/platform:
https://www.home-assistant.io/components/logger/

Description of problem:
When examing huawei_lte component, I've started debugging somehow unexpected behavior in this line: https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/huawei_lte/__init__.py#L83 . I was expecting this statement to be false when logger set to warn ( or anything higher than debug ).
That was not a case. _LOGGER has default level of NOSET which causes this statement to be true. After that I've searched for other examples, to see if there is some error in huawei_lte component and for example history component has the same logic ( https://github.com/home-assistant/home-assistant/blob/2c07bfb9e0ac03fdf74b83152f4511ed7f7104b0/homeassistant/components/history/__init__.py#L223 ), in few places.
All of them are being executed, with logger level warn. Debug messages are not being printed as expected, but the code inside that ifs ( that I believe should not be executed when no doing debug logging in HA ), is still being executed. In that samples its rather small code, and doesn't hurt performance, so maybe that's small bug.
After seeing that such if is being used in other places, which potentially can believe it wont be executed, until debug is set for logger. That can cause some troubles.

Problem-relevant configuration.yaml entries and (fill out even if it seems unimportant):

logger:
  default: warn

Additional information:
Not sure how it could be fixed, maybe all loggers should inherit global logger level ?

@zewelor zewelor changed the title Components logger weird Components logger doesn't respect global logger level May 11, 2019
@zewelor zewelor changed the title Components logger doesn't respect global logger level Components logger doesn't respect default logger level May 11, 2019
@awarecan
Copy link
Contributor

This is misunderstanding/misuse of HA logger component. HA's logger level is not set the logger's level, but add a log filter. Therefore _LOGGER.isEnabledFor(logging.DEBUG) always return True.

https://github.com/home-assistant/home-assistant/blob/b2a1204bc5fe8006a5343248e769f2e5194798ea/homeassistant/components/logger/__init__.py#L107-L108

@ghost
Copy link

ghost commented May 15, 2019

Hey there @scop, mind taking a look at this issue as its been labeled with a integration (huawei_lte) you are listed as a codeowner for? Thanks!

This is a automatic comment generated by codeowners-mention to help ensure issues and pull requests are seen by the right people.

@scop
Copy link
Member

scop commented Jun 2, 2019

If there's a clean way to detect whether debug messages will actually be output, I'm fine with changing huawei_lte to use it. Other than that, I have some plans that if/once materialized, will take a completely different approach to the hunk of code in question. So if there's no clean fix for the current issue, I'm inclined to leave it as is for the time being.

@stale
Copy link

stale bot commented Aug 31, 2019

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates.
Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍
This issue now has been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Aug 31, 2019
@scop scop removed the stale label Sep 3, 2019
@scop scop mentioned this issue Sep 16, 2019
9 tasks
scop added a commit that referenced this issue Oct 24, 2019
* Modernization rework

- config entry support, with override support from huawei_lte platform in YAML
- device tracker entity registry support
- refactor for easier addition of more features
- internal code cleanups

* Remove log level dependent subscription/data debug hack

No longer needed, because pretty much all keys from supported
categories are exposed as sensors.

Closes #23819

* Upgrade huawei-lte-api to 1.4.1

https://github.com/Salamek/huawei-lte-api/releases

* Add support for access without username and password

* Use subclass init instead of config_entries.HANDLERS

* Update huawei-lte-api to 1.4.3 (#27269)

* Convert device state attributes to snake_case

* Simplify scanner entity initialization

* Remove not needed hass reference from Router

* Return explicit None from unsupported old device tracker setup

* Mark unknown connection errors during config as such

* Drop some dead config flow code

* Run config flow sync I/O in executor

* Parametrize config flow login error tests

* Forward entry unload to platforms

* Async/sync fixups

* Improve data subscription debug logging

* Implement on the fly add of new and tracking of seen device tracker entities

* Handle device tracker entry unload cleanup in component

* Remove unnecessary _async_setup_lte, just have code in async_setup_entry

* Remove time tracker on unload

* Fix to not use same mutable default subscription set for all routers

* Pylint fixes

* Remove some redundant defensive device tracker code

* Add back explicit get_scanner None return, hush pylint

* Adjust approach to set system_options on entry create

* Enable some sensors on first add instead of disabling everything

* Fix SMS notification recipients default value

* Add option to skip new device tracker entities

* Fix SMS notification recipient option default

* Work around pylint-dev/pylint#3202

* Remove unrelated type hint additions

* Change async_add_new_entities to a regular function

* Remove option to disable polling for new device tracker entries
thomasgermain pushed a commit to thomasgermain/home-assistant that referenced this issue Nov 16, 2019
* Modernization rework

- config entry support, with override support from huawei_lte platform in YAML
- device tracker entity registry support
- refactor for easier addition of more features
- internal code cleanups

* Remove log level dependent subscription/data debug hack

No longer needed, because pretty much all keys from supported
categories are exposed as sensors.

Closes home-assistant#23819

* Upgrade huawei-lte-api to 1.4.1

https://github.com/Salamek/huawei-lte-api/releases

* Add support for access without username and password

* Use subclass init instead of config_entries.HANDLERS

* Update huawei-lte-api to 1.4.3 (home-assistant#27269)

* Convert device state attributes to snake_case

* Simplify scanner entity initialization

* Remove not needed hass reference from Router

* Return explicit None from unsupported old device tracker setup

* Mark unknown connection errors during config as such

* Drop some dead config flow code

* Run config flow sync I/O in executor

* Parametrize config flow login error tests

* Forward entry unload to platforms

* Async/sync fixups

* Improve data subscription debug logging

* Implement on the fly add of new and tracking of seen device tracker entities

* Handle device tracker entry unload cleanup in component

* Remove unnecessary _async_setup_lte, just have code in async_setup_entry

* Remove time tracker on unload

* Fix to not use same mutable default subscription set for all routers

* Pylint fixes

* Remove some redundant defensive device tracker code

* Add back explicit get_scanner None return, hush pylint

* Adjust approach to set system_options on entry create

* Enable some sensors on first add instead of disabling everything

* Fix SMS notification recipients default value

* Add option to skip new device tracker entities

* Fix SMS notification recipient option default

* Work around pylint-dev/pylint#3202

* Remove unrelated type hint additions

* Change async_add_new_entities to a regular function

* Remove option to disable polling for new device tracker entries
tetienne pushed a commit to tetienne/home-assistant that referenced this issue Nov 22, 2019
* Modernization rework

- config entry support, with override support from huawei_lte platform in YAML
- device tracker entity registry support
- refactor for easier addition of more features
- internal code cleanups

* Remove log level dependent subscription/data debug hack

No longer needed, because pretty much all keys from supported
categories are exposed as sensors.

Closes home-assistant#23819

* Upgrade huawei-lte-api to 1.4.1

https://github.com/Salamek/huawei-lte-api/releases

* Add support for access without username and password

* Use subclass init instead of config_entries.HANDLERS

* Update huawei-lte-api to 1.4.3 (home-assistant#27269)

* Convert device state attributes to snake_case

* Simplify scanner entity initialization

* Remove not needed hass reference from Router

* Return explicit None from unsupported old device tracker setup

* Mark unknown connection errors during config as such

* Drop some dead config flow code

* Run config flow sync I/O in executor

* Parametrize config flow login error tests

* Forward entry unload to platforms

* Async/sync fixups

* Improve data subscription debug logging

* Implement on the fly add of new and tracking of seen device tracker entities

* Handle device tracker entry unload cleanup in component

* Remove unnecessary _async_setup_lte, just have code in async_setup_entry

* Remove time tracker on unload

* Fix to not use same mutable default subscription set for all routers

* Pylint fixes

* Remove some redundant defensive device tracker code

* Add back explicit get_scanner None return, hush pylint

* Adjust approach to set system_options on entry create

* Enable some sensors on first add instead of disabling everything

* Fix SMS notification recipients default value

* Add option to skip new device tracker entities

* Fix SMS notification recipient option default

* Work around pylint-dev/pylint#3202

* Remove unrelated type hint additions

* Change async_add_new_entities to a regular function

* Remove option to disable polling for new device tracker entries
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants