-
Notifications
You must be signed in to change notification settings - Fork 7
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
Loguru for logging. #71
Conversation
Should we switch to the |
"level": "DEBUG", | ||
}, | ||
}, | ||
"loguru": { |
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.
I'd propose to change the config structure.
I propose to move format definition in a separate key instead of function _handler_configuration
Like so:
"loguru" : {
"formats": {
"ch-backup": "{time:YYYY-MM-DD H:m:s,SSS} {process.name:11} {process.id:5} [{level:8}] {extra[logger_name]}: {message}",
},
"handlers": {
"ch-backup": {
"sink": "/var/log/ch-backup/ch-backup.log",
"level": "DEBUG",
"format": "ch-backup"
# "enqueue": True # don't define this but set all handlers in code as enqueued without option
}
#...
}
#...
}
ch_backup/config.py
Outdated
"urllib3.connectionpool", "/var/log/ch-backup/boto.log", "DEBUG" | ||
), | ||
], | ||
"activation": [ |
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.
Let's remove this from config. It may be confusing what this option for.
ch_backup/logging.py
Outdated
frame = frame.f_back | ||
depth += 1 | ||
|
||
try: |
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.
Let's remove this and put bind(logger_name=record.name)
to pass original logger name to handlers
ch_backup/clickhouse/disks.py
Outdated
@@ -184,7 +184,7 @@ def _copy_dir(from_disk: str, from_path: str, to_disk: str, to_path: str) -> Non | |||
|
|||
|
|||
def _exec(command: str, common_args: List[str], command_args: List[str]) -> List[str]: | |||
logger = logging.getLogger("clickhouse-disks") | |||
logger = ch_logging.getLogger("clickhouse-disks") |
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.
Rename ch_logging to logging ?
ch_backup/config.py
Outdated
|
||
|
||
def _as_seconds(t: str) -> int: | ||
return int(parse_timespan(t)) | ||
|
||
|
||
def _handler_configuration(name: str, sink: str, level: str) -> dict: |
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.
Is it needed yet ?
In this PR not. |
Changed the
logging
module tologuru
to provide process-safe logging.