Migrate print calls to logging #1083
Conversation
wwwillchen
force-pushed
the
print-to-logging
branch
from
2a9b636 to
03e3867
Compare
There was a problem hiding this comment.
Nice. I definitely agree using logging is better than the print statements.
There are a few cases where we still uses print statements. I'm curious when it's important to keep the print statement versus using the logger. For example we have mesop/warn.py. Should that just become a logger.warning. Or are those cases special enough to warrant a specific colored print message. But then we also have a case of the CSP error message, which I think makes sense to really format and highlight that to the user.
mesop/env/env.py
Outdated
|
|
||
| EXPERIMENTAL_EDITOR_TOOLBAR_ENABLED = ( | ||
| os.environ.get("MESOP_EXPERIMENTAL_EDITOR_TOOLBAR", "false").lower() == "true" | ||
| ) | ||
|
|
||
| if EXPERIMENTAL_EDITOR_TOOLBAR_ENABLED: | ||
| print("Experiment enabled: EXPERIMENTAL_EDITOR_TOOLBAR_ENABLED") | ||
| logging.info("Experiment enabled: EXPERIMENTAL_EDITOR_TOOLBAR_ENABLED") |
There was a problem hiding this comment.
I think this one is in server.py already. And I think you mentioned this wouldn't show up.
There was a problem hiding this comment.
oops - you're right - forgot to remove this - done.
good call on warn.py - I migrated that to logger.warning, but kept the color formatting so it still stands out in the terminal. For CSP, yeah, I think keeping it as a print is better due to the formatting. tbh, this is an area I've never been super sure about because Mesop as a CLI, print is better vs. Mesop as a library, logging is better. |
This conforms to more standard Python practices and enables apps that use Mesop to customize the logs generated by Mesop (e.g. ignore info logs).
This moves the print statements from env.py into server.py because logging calls need to happen after the main function is called (see: abseil/abseil-py#194 (comment))