Skip to content
This repository was archived by the owner on Jul 10, 2024. It is now read-only.

Logging fixes and improvements #103

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

Logging fixes and improvements #103

wants to merge 1 commit into from

Conversation

scop
Copy link
Contributor

@scop scop commented Dec 29, 2018

See individual commit messages for more details.

@Ape
Copy link
Owner

Ape commented Feb 28, 2019

I cherry picked the first commit, but I think I am not going to merge the other one.

So lib users can control it without affecting everything else.
@scop
Copy link
Contributor Author

scop commented Mar 8, 2019

Can I ask why not?

For example, samsungctl is used within Home Assistant's samsungtv component. It logs in my opinion too verbosely, and because it's using the root logger, it's not trivial to even find out where the too verbose log messages originate from.

With the current setup, there's pretty much nothing that can sanely be done about either issue. Having the remaining commit of this PR applied would fix the logger identity issue, and empower users to tweak the logging as they see fit without affecting the entire Home Assistant setup, with no draw backs I'm aware of.

@Olen
Copy link

Olen commented Sep 10, 2019

I agree with scop on this. What is the problem with using package specific logging?
Please merge, as it makes filtering of logs in other apps much easier.

@foreign-sub
Copy link

Hi, as an other Home Assistant user, i have been wondering for some times why my log was filled with lots of useless data, a google search quickly helped me to identify the culprit only to discover a fix has been ready to commit for months but has never been merged. I would even go further and turn every single message to debug level. There is really no point being so verbose about the API activity if it just works as intended, please reconsider merging this PR. This will clean up hundreds of log files and in the meantime reduce global warming. You can help us save the planet !
Thank you for your assistance and creating this api.

@connesc
Copy link

connesc commented Apr 9, 2020

I would also love to see this merged, for exactly the same reasons as above.
BTW, thanks for your work @Ape! 👍

@kleinc80
Copy link

kleinc80 commented Jan 2, 2022

Hi there, Happy new Year everyone! I know the PR and conversation are quite old but I'll give it a try. :-)
I'm also using this nice piece of work from @Ape in my HomeAssistant and for the reasons mentioned I'd love to see @scop's PR merged! Cheers!

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

Successfully merging this pull request may close these issues.

6 participants