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

Use logging instead of print() #214

Closed
nocalla opened this issue May 25, 2018 · 4 comments
Closed

Use logging instead of print() #214

nocalla opened this issue May 25, 2018 · 4 comments
Assignees
Projects

Comments

@nocalla
Copy link
Member

nocalla commented May 25, 2018

Other feature request

Is your feature request related to a problem? Please describe.
It seems to not be best practice to use print(). A better option is to use logging. This will also allow us to improve debugging if needs be, as well as providing scope for higher priority warnings etc.

Describe the solution you'd like
Let's implement logging instead of our print calls.

Describe alternatives you've considered
We could keep the print as-is, but it's annoying to work around and it would be nice to have the debugging features logging provides.

Additional context
Some references about logging:

@nocalla nocalla self-assigned this May 25, 2018
@nocalla nocalla added this to Candidates in 1.1.0 via automation May 25, 2018
@nocalla nocalla moved this from Candidates to To do in 1.1.0 May 25, 2018
@nocalla
Copy link
Member Author

nocalla commented May 25, 2018

I've merged this into develop with #215 as it didn't break anything. I'd like help improving how the logging works though, particularly allowing the test files to not use the INFO lines for a cleaner test output. @toyg - do you have any feedback here?

@toyg
Copy link
Collaborator

toyg commented May 25, 2018

Step 1: the logging level must be set in a config file, not in source. The simplest way to achieve this is to have a config parameter that can store a value (e.g. LogLevel = INFO), then in code something like:

basicConfig(... , level=getattr(logging,your_param))

(you might want to have a check for acceptable values before: if your_param not in ['INFO','DEBUG'...]: your_param='INFO')

Step 2: if you want to do something more complicated/precise, have a look at dictConfig().

Step 3: you can have a simple environment variable (say DEBUG=1) and depending on that value, switch between basicConfig and dictConfig, so you can e.g. pass a complicated config for debug/testing purposes, and a simple one for production default. Or just keep all the logging machinery in a separate config file (like a JSON) and use dictConfig all the time, providing different files when required.

I'm not a test guru, I'm sure there are many other ways to skin this particular cat, but the ideas above are the simplest strategies and rely on simple stdlib modules.

@nocalla
Copy link
Member Author

nocalla commented Aug 6, 2018

So where this stands now is that we need to add a switch for the different logging levels and assign priorities to each message in the code.

@nocalla
Copy link
Member Author

nocalla commented Oct 15, 2018

I'm going to mark this as done and create a separate issue for creating filters etc.

@nocalla nocalla closed this as completed Oct 15, 2018
1.1.0 automation moved this from To do to Done Oct 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
1.1.0
  
Done
Development

No branches or pull requests

2 participants