Bugfix/prod 442 missing rollover logs results in unknown error message #17
Conversation
rmoneys
left a comment
rmoneys
left a comment
There was a problem hiding this comment.
Awesome! This should really help us understand prediction errors in Datadog: https://p.datadoghq.com/sb/9275ad12-494c-11ec-a393-da7ad0900002-dbf4f29a699254f7c20206a2a7b54af3
| def get_exception_response(self) -> dict: | ||
| """ | ||
| Used to get an example response, which will be used for | ||
| the swagger documentation. |
There was a problem hiding this comment.
I think it better to keep the log parser and prediction models agnostic about the context in which they are used. Specifically, they shouldn't be concerned with the design of a web API.
There was a problem hiding this comment.
I see what you mean. Does not including this inhibit any functionality in the backend when these types of exceptions are thrown?
There was a problem hiding this comment.
Also, are you suggesting to delete just this comment, or the method entirely?
There was a problem hiding this comment.
Nothing comes to mind as to how the back end might be limited without this. Unless you have plans for using this method we can probably remove it.
| class LogSubmissionException(SyncParserException): | ||
| """ | ||
| This Exception is for malformed log submission | ||
| """ |
There was a problem hiding this comment.
Since these are also ValueErrors it'd be good to inherit from that too (e.g. https://github.com/pydantic/pydantic/blob/c22fe9a8f379fc0342345109f7451d98dc433a2d/pydantic/error_wrappers.py#L50), tying this in to the hierarchy: https://docs.python.org/3/library/exceptions.html#exception-hierarchy
|
Kudos, SonarCloud Quality Gate passed!
|
gorskysd
deleted the
bugfix/PROD-442-missing-rollover-logs-results-in-unknown-error-message
branch
Switched most of the ValueError exceptions into SyncLogSubmission exceptions, which will get passed through to the user. These are often user errors, such as a missing rollover logs or extraneous files in the submission.