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][Minor] Remove unreachable code path for ray_namespace. #25305

Merged
merged 10 commits into from
Jul 13, 2022

Conversation

jovany-wang
Copy link
Contributor

@jovany-wang jovany-wang commented May 31, 2022

Why are these changes needed?

The job level ray_namespace will be set to task spec when creating an actor without an explicitly specifying namespace for actor. Therefore, in the gcs actor manager, the ray_namespace in task spec shouldn't be empty.
This PR remove the unreachable code path which is using to get the namespace from a local cache.

Related issue number

Checks

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

@jovany-wang jovany-wang marked this pull request as ready for review May 31, 2022 07:09
@jovany-wang jovany-wang changed the title [Testing] Code clean [Core][Minor] Remove unreachable code path for ray_namespace. Jun 3, 2022
@jovany-wang
Copy link
Contributor Author

jovany-wang commented Jun 22, 2022

The cpp worker test fails due to we haven't supported anonymous namespace. @larrylian @WangTaoTheTonic Do you have plan to support it?

@WangTaoTheTonic
Copy link
Contributor

@jovany-wang Sounds not hard. Will support it ASAP.

@jovany-wang jovany-wang removed the do-not-merge Do not merge this PR! label Jul 12, 2022
@jovany-wang
Copy link
Contributor Author

The cpp namespace was merge #26327 (comment)

So this PR could be merged once CI passes.

@jovany-wang jovany-wang merged commit 12d038b into ray-project:master Jul 13, 2022
@jovany-wang jovany-wang deleted the minor-change branch July 13, 2022 04:34
Stefan-1313 pushed a commit to Stefan-1313/ray_mod that referenced this pull request Aug 18, 2022
…oject#25305)

The job level ray_namespace will be set to task spec when creating an actor without an explicitly specifying namespace for actor. Therefore, in the gcs actor manager, the ray_namespace in task spec shouldn't be empty.
This PR remove the unreachable code path which is using to get the namespace from a local cache.

Signed-off-by: Stefan van der Kleij <[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.

3 participants