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 :(

…and task detail pages

Signed-off-by: Alan Guo <aguo@anyscale.com>
@Copilot Copilot AI review requested due to automatic review settings July 2, 2025 23:04
Copy link
Contributor

@Copilot 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
Collaborator

@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 <aguo@anyscale.com>
Signed-off-by: Alan Guo <aguo@anyscale.com>
Signed-off-by: Alan Guo <aguo@anyscale.com>
@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
Collaborator

@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 <aguo@anyscale.com>
Signed-off-by: ChanChan Mao <chanchanmao1130@gmail.com>
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