-
Notifications
You must be signed in to change notification settings - Fork 8
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
MaintenanceOngoingException #1573
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.
Apart from the minor docstring change suggestion, I like the solution. I produces this in the container log:
| 172.18.0.6 - - [08/Nov/2024 13:55:36] "GET /api/v1/proj/list HTTP/1.1" 503 -
and in the actions.log:
{"action": {"message": "Maintenance of DDS is ongoing.", "resource": "/api/v1/user/info", "project": null, "user": "None (172.18.0.6)", "response": "503 Service Unavailable", "event": "exception.MaintenanceOngoingException", "logger": "actions", "level": "warning", "timestamp": "2024-11-08T13:55:24.950844Z"}}
Co-authored-by: Valentin Georgiev <[email protected]>
@rv0lt just a reminder not to merger despite of my approval |
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.
Want / need to weigh in, will review.
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 like it! Question though regarding the use of flask.logging
as opposed to the regular logging
library -- are there no filters etc in the standard logging library?
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 asked in my previous comment if logging
didn't have a filter, and now I have checked and flask.logging
seems to just use the standard logging library? So I tried just using the regular logging library and it seems to be working exactly the same? So then we should be able to remove flask.logging
, unless I'm missing something.
I will test again your solution then! |
Co-authored-by: Ina Odén Österbo <[email protected]>
Co-authored-by: Ina Odén Österbo <[email protected]>
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.
All good to merge!
Read this before submitting the PR
If there is a field which you are unsure about, enter the edit mode of this description or go to the PR template; There are invisible comments providing descriptions which may be of help.
1. Description / Summary
Add a filter in order to avoid logging out the Maintenace mode expcetion.
The issue is that, every time that a 500 error is generated (SERVER ERROR) it is reported as an exception in the logs in the backend (in contrast with 400 error which are not reported as such).
A maintenance mode error should be, by definition, a 503 error. Therefore, to avoid reporting it, we will filter out in the logs.
2. Jira task / GitHub issue
Link to the github issue or add the Jira task ID here.
3. Type of change
What type of change(s) does the PR contain?
Check the relevant boxes below. For an explanation of the different sections, enter edit mode of this PR description template.
4. Additional information
master
branch: _If checked, read the release instructions5. Actions / Scans
Check the boxes when the specified checks have passed.
For information on what the different checks do and how to fix it if they're failing, enter edit mode of this description or go to the PR template.