-
Notifications
You must be signed in to change notification settings - Fork 209
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
Make 'Broken' workers show as 'Unavailable' instead #6073
Make 'Broken' workers show as 'Unavailable' instead #6073
Conversation
@@ -767,7 +767,8 @@ sub _check_system_utilization ( | |||
return undef unless $threshold && @$load >= 3; | |||
# look at the load evolution over time to react quick enough if the load | |||
# rises but accept a falling edge | |||
return "The average load (@$load) is exceeding the configured threshold of $threshold." | |||
return | |||
"The average load (@$load) is exceeding the configured threshold of $threshold. The worker will temporarily not accept new jobs until the load is lower again." |
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.
one comment quite off the PR. this seems to be displayed as a info next to the status. I wonder why we do not display the load as another key:value field in the page.
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.
you're right that might be a good idea 👍
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.
not sure. Maybe check with others too. for me makes sense but maybe there is a reason
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.
The load is not checked every time the worker details page is shown. The worker sends this info via websocket if the load is too high, and it gets saved into the worker table in the database.
b45a245
to
7a7a74d
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6073 +/- ##
=======================================
Coverage 98.98% 98.98%
=======================================
Files 395 395
Lines 39508 39508
=======================================
Hits 39108 39108
Misses 400 400 ☔ View full report in Codecov by Sentry. |
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.
The "Broken" in the dropdown in the worker list also needs to be changed.
7a7a74d
to
93d89ff
Compare
This commit displays "Broken" workers as instead "Unavailable", as the previous wording was e.g. not including workers which where unavailable due to 'too high load'. For the same reason the 'Error' title of the worker states hover popup is replaced with 'Details' and the 'too high load' output is enhanced. Related Ticket: https://progress.opensuse.org/issues/164084
93d89ff
to
bff1577
Compare
This PR displays "Broken" workers as instead "Unavailable", as the previous wording was e.g. not including workers which where unavailable due to 'too high load'.
It also replaces the 'Error' title of the worker states hover popup with 'Details' and enhances the 'too high load' output.
Example:
Related Ticket: https://progress.opensuse.org/issues/164084