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.

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

[core][dashboard] Add Pg Id and Format Required Resources in /logical/actors Response #47754

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

Conversation

MengjinYan
Copy link
Collaborator

@MengjinYan MengjinYan commented Sep 19, 2024

Problem to Solve

In issue #45658, we found that the placement group id and the required resources are not displayed correctly in the actor UI tab in the dashboard. To be specific:

  • The "Placement group ID" column content is missing
  • The "Required resources" column shows resources with pg ids and bundle entries as well
Screenshot 2024-09-18 at 9 49 27 AM

Root Cause

With investigation, we found the root cause of the issue is that:

  • The dashboard calls "/logical/actors" to get the actor information to display
  • The response content of the above REST API doesn't contain the placement group id field and the required resources are not formatted correctly
  • Therefore, the dashboard fails to the display the corresponding content correctly

Changes in the PR

  • Include placementGroupId in the keys to be translated/preserved in the actor_table_data_to_dict function so that the pg id can be included in the content of the REST API response
  • Leverage the parse_pg_formatted_resources_to_original function to reformatted the required resources before sending the REST API response
  • Test logic added in test_actors tests to verify the logic of setting the placement group id as well as the correctly formatted required resources

Related issue number

#45658

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

… the REST API response that the dashboard calls

Signed-off-by: Mengjin Yan <[email protected]>
Signed-off-by: Mengjin Yan <[email protected]>
Signed-off-by: Mengjin Yan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant