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

Setup Python logging + convert print statements to logger calls #557

Closed
wants to merge 1 commit into from

Conversation

dbast
Copy link
Contributor

@dbast dbast commented May 15, 2024

  • Setup the standard Python logging module via main.py with cli argument so that different log levels can be chosen.
  • Move the main.py into ./src/seedsigner folder, to be able to importable for unit-testing.
  • Create a symlink from./src/main.py -> ./src/seedsigner/main.py so that seedsigner can be started without any changes to seedsigner-os
  • Add uni-tests for the main.py regarding the default log level and changing the log level.
  • Converts all existing print statements to logger.info calls (INFO is the default log level)
  • Produces tab aligned log message like 2024-05-15 17:21:45.385 INFO: Starting Seedsigner with: {'loglevel': 'INFO'}
  • The sub seconds display + the option to choose the loglevel help to debug the application and also see how long certain code needs

There are many many commented out print statements that could be converted to logger.debug(...) calls and thus be shown additionally via running main.py -l DEBUG during dev work, but that is for next PRs as all those commented out print statements can be potential broken by referencing non existing variables etc.

This is a move towards more Pythonic ways of doing: The logging framework with its central configurability, options to set levels, the ability to activate certain log lines by choosing a different log level (without un-commenting/commenting print statements), the possibility to the see how much times is between log calls (without extra instrumenting the code) etc is all in all very convenient.

The help output on cli:

usage: main.py [-h] [-l {CRITICAL,FATAL,ERROR,WARN,WARNING,INFO,DEBUG,NOTSET}]

options:
  -h, --help            show this help message and exit
  -l {CRITICAL,FATAL,ERROR,WARN,WARNING,INFO,DEBUG,NOTSET}, --loglevel {CRITICAL,FATAL,ERROR,WARN,WARNING,INFO,DEBUG,NOTSET}
                        Set the log level (default: INFO)

Description

Describe the change simply. Provide a reason for the change.

Include screenshots of any new or modified screens (or at least explain why they were omitted)

This pull request is categorized as a:

  • New feature
  • Bug fix
  • Code refactor
  • Documentation
  • Other

Checklist

  • I’ve run pytest and made sure all unit tests pass before sumbitting the PR

If you modified or added functionality/workflow, did you add new unit tests?

  • No, I’m a fool
  • Yes
  • N/A

I have tested this PR on the following platforms/os:

Note: Keep your changes limited in scope; if you uncover other issues or improvements along the way, ideally submit those as a separate PR. The more complicated the PR the harder to review, test, and merge.

* Setup the standard Python logging module via main.py with cli argument
  so that different log levels can be chosen.
* Move the main.py into ./src/seedsigner folder, to be able to importable
  for unit-testing.
* Create a symlink from./src/main.py -> ./src/seedsigner/main.py so that
  seedsigner can be started without any changes to seedsigner-os
* Add uni-tests for the main.py regarding the default log level and
  changing the log level.
* Converts all existing print statements to `logger.info` calls (INFO
  is the default log level)
* Produces tab aligned log message like
  `2024-05-15 17:21:45.385 INFO:	Starting Seedsigner with: {'loglevel': 'INFO'}`
* The sub seconds display + the option to choose the loglevel help to
  debug the application and also see how long certain code needs


There are many many commented out `print` statements in the code base that
could be converted to `logger.debug(...)` calls and thus be shown
additionally via running `main.py -l DEBUG`, but that is for next PRs as
all those commented out print statements can be potential broken by
referencing non existing variables etc.

This is a move towards more Pythonic ways of doing: The logging framework
with its central configurability, options to set levels, the ability to
activate certain log lines by choosing a different log level (without
un-commenting/commenting print statements), the possibility to the see
how much times is between log calls (without extra instrumenting the code)
etc is all in all very convenient.

The help output on cli:
```
usage: main.py [-h] [-l {CRITICAL,FATAL,ERROR,WARN,WARNING,INFO,DEBUG,NOTSET}]

options:
  -h, --help            show this help message and exit
  -l {CRITICAL,FATAL,ERROR,WARN,WARNING,INFO,DEBUG,NOTSET}, --loglevel {CRITICAL,FATAL,ERROR,WARN,WARNING,INFO,DEBUG,NOTSET}
                        Set the log level (default: INFO)
```
@dbast dbast marked this pull request as draft May 15, 2024 15:44
@dbast
Copy link
Contributor Author

dbast commented May 15, 2024

breaks the tests... closing until fixed.

@dbast dbast closed this May 15, 2024
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

Successfully merging this pull request may close these issues.

1 participant