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.

Sign up for GitHub

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

Jump to bottom

Use vendored cloudpickle for Ray client #14576

Merged
merged 2 commits into from
Mar 10, 2021
Merged

Use vendored cloudpickle for Ray client #14576

merged 2 commits into from
Mar 10, 2021

Conversation

ericl
Copy link
Contributor

@ericl ericl commented Mar 9, 2021 •
edited
Loading

Switch to using the vendored cloudpickle, which ensures better serialization compatibility and avoids the extra dependency.

@ericl
Copy link
Contributor Author

ericl commented Mar 10, 2021

Hmm this seems to pass all tests. @barakmich if you remember the previous issue with the vendored cloudpickle.

@ericl ericl changed the title [WIP] Use vendored cloudpickle for Ray client Use vendored cloudpickle for Ray client Mar 10, 2021
@ericl ericl added the tests-ok The tagger certifies test failures are unrelated and assumes personal liability. label Mar 10, 2021
pcmoritz
pcmoritz approved these changes Mar 10, 2021
View reviewed changes
Copy link
Contributor

@pcmoritz pcmoritz left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for doing this :)

@ericl
Copy link
Contributor Author

ericl commented Mar 10, 2021

Ok, let's see what terrible thing happens when we merge

@ericl ericl merged commit dcb22af into ray-project:master Mar 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pcmoritz pcmoritz pcmoritz approved these changes

Assignees

@pcmoritz pcmoritz

Labels
tests-ok The tagger certifies test failures are unrelated and assumes personal liability.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ericl @pcmoritz