-
Notifications
You must be signed in to change notification settings - Fork 7k
Fix/ray serve test state api usage #56948
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
Fix/ray serve test state api usage #56948
Conversation
Signed-off-by: Jason <[email protected]>
Instead of fetching actors from the internal state API / migrating it to the common module, simply reference the public state API that relies on the dashboard server. Signed-off-by: Jason <[email protected]>
Signed-off-by: Jason <[email protected]>
Signed-off-by: Jason <[email protected]>
Signed-off-by: Jason <[email protected]>
Signed-off-by: Jason <[email protected]>
Signed-off-by: Jason <[email protected]>
Signed-off-by: Jason <[email protected]>
Signed-off-by: Jason <[email protected]>
Fixes the runtime context state submodule loading bug by importing the state API module as STATE at the top of the file. Uses similar safety precaution in the dataset.py file since these were the only two public API spots recently changed (not isolated to just tests). Signed-off-by: Jason <[email protected]>
Signed-off-by: Jason <[email protected]>
The dashboard wasn't being initialized before unit tests for shutdown_only fixtures since the params get ignored, causing some unit tests to fail. Signed-off-by: Jason <[email protected]>
Signed-off-by: Jason <[email protected]>
Signed-off-by: Jason <[email protected]>
Signed-off-by: Jason <[email protected]>
Signed-off-by: Jason <[email protected]>
Signed-off-by: Jason <[email protected]>
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.
Code Review
This pull request addresses a bug in the test_actor_summary unit test within python/ray/serve/tests/test_metrics.py. The test was using dictionary-style access (actor['class_name']) on objects returned by list_actors, which now returns a list of ActorState dataclass objects. The fix correctly changes this to attribute-style access (actor.class_name). This change is correct and aligns the test with the current ray.util.state API, resolving the issue.
|
@edoakes Who's a good reviewer to look at this? It's an extremely small change that fell out of the PR you reviewed for migrating the state#actors method calls to the public API (list_actors, get_actor). |
|
@zcin @abrarsheikh PTAL |
|
This pull request has been automatically marked as stale because it has not had You can always ask for help on our discussion forum or Ray's public slack channel. If you'd like to keep this open, just leave any comment, and the stale label will be removed. |
|
@jcarlson212 changes look good, mind merging master into your branch, premerge is failing |
## Why are these changes needed? Fixes a unit test that was broken but not running in CI. ## Related issue number Fixes ray-project#53478 ## Checks - [y] I've signed off every commit(by using the -s flag, i.e., `git commit -s`) in this PR. - [y] I've run `scripts/format.sh` to lint the changes in this PR. - [y] 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. - [y] 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 - [y] Unit tests - [ ] Release tests - [ ] This PR is not tested :( --------- Signed-off-by: Jason <[email protected]> Co-authored-by: Edward Oakes <[email protected]> Signed-off-by: xgui <[email protected]>
## Why are these changes needed? Fixes a unit test that was broken but not running in CI. ## Related issue number Fixes #53478 ## Checks - [y] I've signed off every commit(by using the -s flag, i.e., `git commit -s`) in this PR. - [y] I've run `scripts/format.sh` to lint the changes in this PR. - [y] 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. - [y] 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 - [y] Unit tests - [ ] Release tests - [ ] This PR is not tested :( --------- Signed-off-by: Jason <[email protected]> Co-authored-by: Edward Oakes <[email protected]> Signed-off-by: elliot-barn <[email protected]>
## Why are these changes needed? Fixes a unit test that was broken but not running in CI. ## Related issue number Fixes ray-project#53478 ## Checks - [y] I've signed off every commit(by using the -s flag, i.e., `git commit -s`) in this PR. - [y] I've run `scripts/format.sh` to lint the changes in this PR. - [y] 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. - [y] 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 - [y] Unit tests - [ ] Release tests - [ ] This PR is not tested :( --------- Signed-off-by: Jason <[email protected]> Co-authored-by: Edward Oakes <[email protected]>
## Why are these changes needed? Fixes a unit test that was broken but not running in CI. ## Related issue number Fixes ray-project#53478 ## Checks - [y] I've signed off every commit(by using the -s flag, i.e., `git commit -s`) in this PR. - [y] I've run `scripts/format.sh` to lint the changes in this PR. - [y] 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. - [y] 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 - [y] Unit tests - [ ] Release tests - [ ] This PR is not tested :( --------- Signed-off-by: Jason <[email protected]> Co-authored-by: Edward Oakes <[email protected]> Signed-off-by: Aydin Abiar <[email protected]>
## Why are these changes needed? Fixes a unit test that was broken but not running in CI. ## Related issue number Fixes ray-project#53478 ## Checks - [y] I've signed off every commit(by using the -s flag, i.e., `git commit -s`) in this PR. - [y] I've run `scripts/format.sh` to lint the changes in this PR. - [y] 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. - [y] 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 - [y] Unit tests - [ ] Release tests - [ ] This PR is not tested :( --------- Signed-off-by: Jason <[email protected]> Co-authored-by: Edward Oakes <[email protected]> Signed-off-by: Future-Outlier <[email protected]>
Why are these changes needed?
Fixes a unit test that was broken but not running in CI.
Related issue number
Fixes #53478
Checks
git commit -s) in this PR.scripts/format.shto lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/under thecorresponding
.rstfile.