-
-
Notifications
You must be signed in to change notification settings - Fork 8.8k
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
remove idle executors from widget #9177
remove idle executors from widget #9177
Conversation
showing idle executors brings no value. They only blow up the widget. Instead the busy/total number of executors are added behind the agent name when the agent is online. The executor name is kept intentionally to distinguish regular executors from oneoffexecutors
This is pretty nice 👍 One minor issue seems to be that without agents, the widget is empty now, as the header for Built-In Node only shows if you have agents. This may be confusing to (intermediate) users, it looks like nothing can build. (This may even be an opportunity to advertise https://www.jenkins.io/redirect/distributed-builds/ ?) Another possible issue is that the executor number is shown and if you have several, those numbers appear and it's unclear what they're for or why someone should care. Are they exposed anywhere other than thread dumps? If not, would a symbol, or color, to distinguish between normal/heavyweight and lightweight executors work better? (Also, JENKINS-72745 is extremely noticeable now, with a tooltip on the "0/2". Not new here, of course.) |
showing now the same text as when the widget is collapsed, still minor issue that when there are busy executors then the total configured is not shown.
The executor number is also not really interesting so I changed it that only OneOffExecutors (Flyweighttask) use an icon (with tooltip)
That is probably JENKINS-72744 |
Please take a moment and address the merge conflicts of your pull request. Thanks! |
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.
LGTM tested locally
/label ready-for-merge This PR is now ready for merge, after ~24 hours, we will merge it if there's no negative feedback. Thanks! |
Likely caused jenkinsci/acceptance-test-harness#1603 |
I think that this causes a Jelly exception when the docker plugin attempts to edit a cloud definition. Refer to JENKINS-73554 for details. Only visible in weekly releases, not in Jenkins LTS 2.452.3 or Jenkins LTS 2.462.1 |
showing idle executors brings no value. They only blow up the widget. Instead the busy/total number of executors are added behind the agent name when the agent is online.
The executor number is removed, to distinguish regular executors from tasks from asynchronous executions (flyweighttask) the latter get a special symbol.
Adds a new icon size class
icon-xs
so the symbols when agents are launching or are offline has better integration into the surrounding brackets and text).The built-in agent is only shown when it has executors configured or when it is executing an async task that wants to show it's cell
When only a single agent is shown, the busy/total moves to the widget header.
When widget is collapsed, the shown text now distinguishes more different situations.
Before:
After:
Dashboard with several agents:
Dashboard with built-in no executors and one agent:
collapsed
Dashboard with built-in no executors and two agents:
collapsed:
Dashboard with built-in with 2 executors and one agent:
collapsed:
Dashboard with built-in with 2 executors and no agents:
collapsed:
Specific executor
collapsed:
No executors, agents or clouds
Symbols for offline and launching:
Testing done
manual testing
Proposed changelog entries
Proposed upgrade guidelines
N/A
Submitter checklist
Desired reviewers
@mention
Before the changes are marked as
ready-for-merge
:Maintainer checklist