-
Notifications
You must be signed in to change notification settings - Fork 3
Set of small fixes #20
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
Conversation
4aa3180 to
407a6d9
Compare
|
@maretskiy I merged another patch seems like we can close this one |
| except Exception as e: | ||
| LOG.error(("Backend '{}' driver '{}' " | ||
| "error: {}").format(backend, drv_name, e)) | ||
| LOG.error("Backend '{}' driver '{}': {}: {}".format( |
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.
logging itself formats the string and you should not do it explicitly because logging does it according to the specified level and can skip the formatting if it is not necessary. So, instead of
LOG.error("Backend '{}' driver '{}': {}: {}".format(backend, drv_name, type(e), e))
you should do:
LOG.error("Backend '%s' driver '%s': %s: %s", backend, drv_name, type(e), e)
Unfortunately, logging does not support the format style by default.
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 changed all such occurrences in seecloud/availability#24 and other projects as well.
| "*": {"type": "object"} | ||
| } | ||
| }, | ||
| "logging": { |
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 would suggest not to add this functionality right now and do it properly later by dying the appropriate configuration of logging throught fileConfig or dictConfig in oss-lib.
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.
This commit is already merged by another pull request (as dependency) so let's fix this later during integration with oss-lib
No description provided.