Skip to content
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.

Sign up for GitHub

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

Jump to bottom

remove idle executors from widget #9177

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

remove idle executors from widget #9177

wants to merge 10 commits into from

Conversation

mawinter69
Copy link
Contributor

@mawinter69 mawinter69 commented Apr 17, 2024 •
edited

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:

image

After:

Dashboard with several agents:

image

Dashboard with built-in no executors and one agent:

image
image
collapsed
image

Dashboard with built-in no executors and two agents:

collapsed:
image

Dashboard with built-in with 2 executors and one agent:

image
collapsed:
image

Dashboard with built-in with 2 executors and no agents:

image

collapsed:
image

Specific executor

image
collapsed:
image

No executors, agents or clouds

image

Symbols for offline and launching:

image

Testing done

manual testing

Proposed changelog entries

  • remove idle executors from widget

Proposed upgrade guidelines

N/A

Submitter checklist

Edit tasklist title
Beta Give feedback Tasklist Submitter checklist, more options

Delete tasklist

Delete tasklist block?
Are you sure? All relationships in this tasklist will be removed.
  1. The Jira issue, if it exists, is well-described.
    Options
  2. The changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developers, depending on the change) and are in the imperative mood (see examples). Fill in the Proposed upgrade guidelines section only if there are breaking changes or changes that may require extra steps from users during upgrade.
    Options
  3. There is automated testing or an explanation as to why this change has no tests.
    Options
  4. New public classes, fields, and methods are annotated with @Restricted or have @since TODO Javadocs, as appropriate.
    Options
  5. New deprecations are annotated with @Deprecated(since = "TODO") or @Deprecated(forRemoval = true, since = "TODO"), if applicable.
    Options
  6. New or substantially changed JavaScript is not defined inline and does not call eval to ease future introduction of Content Security Policy (CSP) directives (see documentation).
    Options
  7. For dependency updates, there are links to external changelogs and, if possible, full differentials.
    Options
  8. For new APIs and extension points, there is a link to at least one consumer.
    Options

Desired reviewers

@mention

Before the changes are marked as ready-for-merge:

Maintainer checklist

Edit tasklist title
Beta Give feedback Tasklist Maintainer checklist, more options

Delete tasklist

Delete tasklist block?
Are you sure? All relationships in this tasklist will be removed.
  1. There are at least two (2) approvals for the pull request and no outstanding requests for change.
    Options
  2. Conversations in the pull request are over, or it is explicit that a reviewer is not blocking the change.
    Options
  3. Changelog entries in the pull request title and/or Proposed changelog entries are accurate, human-readable, and in the imperative mood.
    Options
  4. Proper changelog labels are set so that the changelog can be generated automatically.
    Options
  5. If the change needs additional upgrade steps from users, the upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the pull request title (see example).
    Options
  6. If it would make sense to backport the change to LTS, a Jira issue must exist, be a Bug or Improvement, and be labeled as lts-candidate to be considered (see query).
    Options

@mawinter69
remove idle executors from widget
110e465 
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
@mawinter69 mawinter69 mentioned this pull request Apr 17, 2024
Open
@daniel-beck
Copy link
Member

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.)

@mawinter69
feedback
65ff1e3 
use an icon for oneoffexecutors (FlyweightTask), instead of using a
number for normal executors (which is also not really interesting).

When there are no agents and no builds running use the same text as when
collapsed to show that nothing is running.
@mawinter69
Copy link
Contributor Author

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/ ?)

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.

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?

The executor number is also not really interesting so I changed it that only OneOffExecutors (Flyweighttask) use an icon (with tooltip)

(Also, JENKINS-72745 is extremely noticeable now, with a tooltip on the "0/2". Not new here, of course.)

That is probably JENKINS-72744

@mawinter69 mawinter69 marked this pull request as ready for review May 2, 2024 06:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@mawinter69 @daniel-beck