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

[Client] Make Client{ObjectRef,ActorRef} subclasses of their server-side counterparts #16110

Merged
merged 5 commits into from
Jun 1, 2021

Conversation

mwtian
Copy link
Member

@mwtian mwtian commented May 27, 2021

Why are these changes needed?

This avoids code duplication and user confusion when handling Client{ObjectRef,ActorRef}. Users just need to know about ObjectRef and ActorID regardless of whether the logic runs in Ray worker or with Ray client.

Compared to current implementations of Client{ObjectRef,ActorRef}, the differences of the new implementations are the additional methods from the parents ObjectRef and ActorID. No additional differences are intended.

Related issue number

Closes #14042

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

@mwtian mwtian added the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label May 27, 2021
@mwtian mwtian changed the title [WIP] Implement ClientObjectRef in cython [WIP] Make ClientObjectRef a subclass of ObjectRef May 27, 2021
@mwtian mwtian force-pushed the ref branch 2 times, most recently from f6e13dc to 10ad72b Compare May 27, 2021 23:44
@mwtian mwtian removed the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label May 28, 2021
@mwtian mwtian changed the title [WIP] Make ClientObjectRef a subclass of ObjectRef [Client] Make Client{ObjectRef,ActorRef} subclasses of their server-side counterparts May 28, 2021
@DmitriGekhtman
Copy link
Contributor

coool, all the buildkite checks pass, will take a closer look soon

python/ray/_raylet.pxd Outdated Show resolved Hide resolved
Add basic unit tests.
@mwtian mwtian requested a review from ijrsvt May 31, 2021 18:11
Copy link
Contributor

@ijrsvt ijrsvt left a comment

Choose a reason for hiding this comment

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

Thank you for adding these tests!!

@AmeerHajAli
Copy link
Contributor

@ericl , can you please review this?

Copy link
Contributor

@ijrsvt ijrsvt left a comment

Choose a reason for hiding this comment

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

Looks good from my perspective! Just want a few more eyes on it before merging!

@ericl
Copy link
Contributor

ericl commented Jun 1, 2021

Looks great, thanks for digging into how to fix this!

@AmeerHajAli AmeerHajAli merged commit f14f197 into ray-project:master Jun 1, 2021
@AmeerHajAli
Copy link
Contributor

Thanks @mwtian !

@mwtian mwtian deleted the ref branch June 2, 2021 02:23
wuisawesome pushed a commit to wuisawesome/ray that referenced this pull request Jun 2, 2021
wuisawesome added a commit that referenced this pull request Jun 2, 2021
wuisawesome pushed a commit that referenced this pull request Jun 2, 2021
mwtian added a commit to mwtian/ray that referenced this pull request Jun 3, 2021
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.

[Client] Provide common ObjectRefType
5 participants