-
-
Notifications
You must be signed in to change notification settings - Fork 769
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
Log handled errors to warning #1926
Conversation
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.
Thanks for the PR!
I left one comment for a log that I would keep on something more severe than info
.
@Ruwann kind reminder to merge 🙏 🙇 |
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.
Thanks @alfechner!
Fixes #1925. Changes proposed in this pull request: - Log handled errors to `warning` instead of `error`. - Log validation errors to `info` because the intent of the log lines in informational. The error is handled by raising a new error. --------- Co-authored-by: Alex Fechner <alex.fechner@stryker.com>
@@ -68,7 +68,7 @@ def _validate(self, body: t.Any) -> t.Optional[dict]: | |||
return self._validator.validate(body) | |||
except ValidationError as exception: | |||
error_path_msg = format_error_with_path(exception=exception) | |||
logger.error( | |||
logger.info( |
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.
It would be nice to configure this, as I'd quite like it as a warning; but not an error, and it's clear to me from this that many people may have different use-cases. I Know I can redirect a log-stream.
…#1926. (#1935) @RobbeSneyders, @Ruwann sorry for this, it seems like I've sneaked in a duplicate log statement in PR #1926 🙇♂️ --------- Co-authored-by: Alex Fechner <alex.fechner@stryker.com>
Fixes #1925.
Changes proposed in this pull request:
warning
instead oferror
.info
because the intent of the log lines in informational. The error is handled by raising a new error.