Skip to content

Conversation

@alanwguo
Copy link
Contributor

@alanwguo alanwguo commented Jul 2, 2025

Why are these changes needed?

Follow-up to #53423

Missed a few places in the UI.
Also updates placement group tables to use the same code preview component as the actor and tasks tables.

Placement group table
Screenshot 2025-07-02 at 4 00 53 PM
Screenshot 2025-07-02 at 4 00 56 PM

Actor detail
Screenshot 2025-07-02 at 4 01 05 PM

Task detail
Screenshot 2025-07-02 at 4 01 19 PM

Related issue number

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
    • I've added any new APIs to the API Reference. For example, if I added a
      method in Tune, I've added it in doc/source/tune/api/ under the
      corresponding .rst file.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

Copilot AI review requested due to automatic review settings July 2, 2025 23:04
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds observability for label selectors across several UI components and updates the placement group tables to use a unified code preview component.

  • Extend the Bundle type with an optional label_selector field.
  • Add “Label Selector” sections in Task and Actor detail pages using CodeDialogButtonWithPreview.
  • Introduce a LabelSelector column/component in the Placement Group table.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
python/ray/dashboard/client/src/type/placementGroup.ts Add label_selector to Bundle type.
python/ray/dashboard/client/src/pages/task/TaskPage.tsx Display label selector in task details.
python/ray/dashboard/client/src/pages/actor/ActorDetail.tsx Display required resources and label selector.
python/ray/dashboard/client/src/components/PlacementGroupTable.tsx Add LabelSelector component and table column.

Copy link
Contributor

@MengjinYan MengjinYan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank for the quick turn-around! I'm not familiar with the UI code to review the exact change, but the change of dashboard UI looks good!

alanwguo added 3 commits July 2, 2025 16:21
Signed-off-by: Alan Guo <[email protected]>
Signed-off-by: Alan Guo <[email protected]>
Signed-off-by: Alan Guo <[email protected]>
@alanwguo alanwguo added the go add ONLY when ready to merge, run all tests label Jul 3, 2025
@alanwguo
Copy link
Contributor Author

alanwguo commented Jul 3, 2025

@MengjinYan ready to merge!

@MengjinYan
Copy link
Contributor

@edoakes Can you help to merge the PR?

Copy link

@bartcheers bartcheers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. Other than the waitFor vs setTimeout there's a small opportunity to DRY up undleResourceRequirements and LabelSelector since they share quite a bit of the logic, but they don't seem exactly the same so might be overdoing it.

await user.type(input, "CREATED");

// Wait for the filter to be applied
await new Promise((resolve) => setTimeout(resolve, 100));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can do something like this with testing-library instead of the timeouts, which should make the test less flaky:

 await waitFor(() => {
      expect(screen.queryByText("pg-987654321")).not.toBeInTheDocument();
      expect(screen.queryByText("pg-555666777")).not.toBeInTheDocument();
    });

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There 3 other places in this PR where you could replace setTimeout for waitFor

@edoakes edoakes merged commit 5fbfc81 into ray-project:master Jul 9, 2025
6 checks passed
ccmao1130 pushed a commit to ccmao1130/ray that referenced this pull request Jul 29, 2025
…and task detail pages (ray-project#54292)

Follow-up to ray-project#53423

Missed a few places in the UI.
Also updates placement group tables to use the same code preview
component as the actor and tasks tables.

Placement group table
![Screenshot 2025-07-02 at 4 00
53 PM](https://github.com/user-attachments/assets/8de97470-abda-4680-b2fb-a4f90add0063)
![Screenshot 2025-07-02 at 4 00
56 PM](https://github.com/user-attachments/assets/a3c37e6f-c9db-4b37-b873-a5fbbd3012d7)

Actor detail
![Screenshot 2025-07-02 at 4 01
05 PM](https://github.com/user-attachments/assets/839cdaea-b441-4380-9c77-4ccb4ebfe563)

Task detail
![Screenshot 2025-07-02 at 4 01
19 PM](https://github.com/user-attachments/assets/aa12461c-a192-4114-a7fd-824613b9c6e6)

---------

Signed-off-by: Alan Guo <[email protected]>
Signed-off-by: ChanChan Mao <[email protected]>
jugalshah291 pushed a commit to jugalshah291/ray_fork that referenced this pull request Sep 11, 2025
…and task detail pages (ray-project#54292)

Follow-up to ray-project#53423

Missed a few places in the UI.
Also updates placement group tables to use the same code preview
component as the actor and tasks tables.

Placement group table
![Screenshot 2025-07-02 at 4 00
53 PM](https://github.com/user-attachments/assets/8de97470-abda-4680-b2fb-a4f90add0063)
![Screenshot 2025-07-02 at 4 00
56 PM](https://github.com/user-attachments/assets/a3c37e6f-c9db-4b37-b873-a5fbbd3012d7)

Actor detail
![Screenshot 2025-07-02 at 4 01
05 PM](https://github.com/user-attachments/assets/839cdaea-b441-4380-9c77-4ccb4ebfe563)

Task detail
![Screenshot 2025-07-02 at 4 01
19 PM](https://github.com/user-attachments/assets/aa12461c-a192-4114-a7fd-824613b9c6e6)

---------

Signed-off-by: Alan Guo <[email protected]>
Signed-off-by: jugalshah291 <[email protected]>
dstrodtman pushed a commit to dstrodtman/ray that referenced this pull request Oct 6, 2025
…and task detail pages (ray-project#54292)

Follow-up to ray-project#53423

Missed a few places in the UI.
Also updates placement group tables to use the same code preview
component as the actor and tasks tables.

Placement group table
![Screenshot 2025-07-02 at 4 00
53 PM](https://github.com/user-attachments/assets/8de97470-abda-4680-b2fb-a4f90add0063)
![Screenshot 2025-07-02 at 4 00
56 PM](https://github.com/user-attachments/assets/a3c37e6f-c9db-4b37-b873-a5fbbd3012d7)

Actor detail
![Screenshot 2025-07-02 at 4 01
05 PM](https://github.com/user-attachments/assets/839cdaea-b441-4380-9c77-4ccb4ebfe563)

Task detail
![Screenshot 2025-07-02 at 4 01
19 PM](https://github.com/user-attachments/assets/aa12461c-a192-4114-a7fd-824613b9c6e6)

---------

Signed-off-by: Alan Guo <[email protected]>
Signed-off-by: Douglas Strodtman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

go add ONLY when ready to merge, run all tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants