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
improve the log configuration with using RotatingFileHandler. #4844
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4844 +/- ##
==========================================
- Coverage 87.85% 87.82% -0.04%
==========================================
Files 160 160
Lines 9735 9740 +5
Branches 1437 1438 +1
==========================================
+ Hits 8553 8554 +1
- Misses 925 928 +3
- Partials 257 258 +1
|
@Gallaecio hi, there is a check , codecov/patch, not successful, is it must be successful then the request can be merged? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work!
The final step would be to cover the changes in tests (which should make the codecov/patch
check pass). In this case it may not be trivial, though. Please, let me know if you get stuck, maybe I can look into it myself when I have some time.
from logging.config import dictConfig | ||
|
||
from logging.handlers import RotatingFileHandler | ||
from twisted.python import log as twisted_log |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💄 Please, keep the empty line separating imports from the standard library from imports from third-party libraries.
@@ -122,7 +122,14 @@ def _get_handler(settings): | |||
filename = settings.get('LOG_FILE') | |||
if filename: | |||
encoding = settings.get('LOG_ENCODING') | |||
handler = logging.FileHandler(filename, encoding=encoding) | |||
if settings.get("LOG_ROTATING") is True: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if settings.get("LOG_ROTATING") is True: | |
if settings.getbool("LOG_ROTATING") is True: |
This makes it possible to set the setting from the command line, where every value is a string.
@@ -154,7 +157,11 @@ These settings can be used to configure the logging: | |||
The first couple of settings define a destination for log messages. If | |||
:setting:`LOG_FILE` is set, messages sent through the root logger will be | |||
redirected to a file named :setting:`LOG_FILE` with encoding | |||
:setting:`LOG_ENCODING`. If unset and :setting:`LOG_ENABLED` is ``True``, log | |||
:setting:`LOG_ENCODING`. If :setting:`LOG_FILE` is set and | |||
:setting:`LOG_ROTATING` is ```True```, the file named |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💄
:setting:`LOG_ROTATING` is ```True```, the file named | |
:setting:`LOG_ROTATING` is ``True``, the file named |
@Gallaecio I made a new PR to pick this up since this is stale |
Add new options about log so that users can set the size and backup count of log file.
Resolves #3628