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

importing Heyoo is messing logging configuration #56

Closed
rbarcelos opened this issue Jan 22, 2023 · 5 comments
Closed

importing Heyoo is messing logging configuration #56

rbarcelos opened this issue Jan 22, 2023 · 5 comments

Comments

@rbarcelos
Copy link

rbarcelos commented Jan 22, 2023

heyoo messes up logging init and it needs to be placed after basicConfig statement as fol;owing:

from flask_executor import Executor

LOG_FILE_NAME = "service.log"
logging.basicConfig(
level=logging.DEBUG,
filename=LOG_FILE_NAME,
filemode="a+",
format="%(asctime)-15s %(levelname)-8s %(message)s",
)

// Unfortunately heyoo messes up logging init, needs to be placed after basicConfig statement
from heyoo import
WhatsApp # pylint: disable=wrong-import-position, wrong-import-order

@Kalebu
Copy link
Contributor

Kalebu commented Jan 24, 2023

Hey @rbarcelos Can you please explain more about the issue with current logging and the your suggestion to fixing it ?

@rbarcelos
Copy link
Author

rbarcelos commented Jan 25, 2023

Disclaimer: I just started playing with Python so no guarantees that my guidance is correct..

Looking at your code (iniit.py), given the way that you are setting up logging, it will be called as soon as I import your module. For some reason, calling logging.basicConfig with my configs after that wont update it.

One possible fix: make logging an optional parameter on WhatsApp ctor. So consumers can passe their instance instead of you creating yours. It aligns with the dependency injection pattern

@soerenetler
Copy link
Collaborator

I tried to look into this a little and find some best practices. https://docs.python.org/3/howto/logging.html#configuring-logging-for-a-library
I think the library should use _logger = logging.getLogger(__name__) and give the specific configuration to the user.
I could look into it further and create a PR.

@Kalebu
Copy link
Contributor

Kalebu commented Feb 14, 2023

I see
Please proceed @soerenetler

@Kalebu
Copy link
Contributor

Kalebu commented Feb 22, 2023

Resolved by #71

@Kalebu Kalebu closed this as completed Feb 22, 2023
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

3 participants