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

Bump Ray client protocol version; fix dataclasses dependency for py 3.6 #14654

Merged
merged 3 commits into from
Mar 12, 2021

Conversation

ericl
Copy link
Contributor

@ericl ericl commented Mar 12, 2021

The addition of the .options(runtime_env) field breaks Ray client protocol compatibility. In addition, the dataclasses module was incorrectly moved to extras; it's needed for Ray client.

@ericl ericl changed the title Bump Ray client protocol version Bump Ray client protocol version ; fix dataclasses dependency for py 3.6 Mar 12, 2021
@ericl ericl changed the title Bump Ray client protocol version ; fix dataclasses dependency for py 3.6 Bump Ray client protocol version; fix dataclasses dependency for py 3.6 Mar 12, 2021
@@ -131,6 +131,7 @@
"click >= 7.0",
"colorama",
"colorful",
"dataclasses; python_version < '3.7'",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@richardliaw did we move this accidentally recently? Python 3.6 used to work (and we have dependend on dataclasses in ray client since last year).

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't recall; though I think we didn't have the python version setup correctly before. dataclasses will break on 3.7.

@edoakes
Copy link
Contributor

edoakes commented Mar 12, 2021

@ericl this won't break every time we add a new runtime_env option, right? just because it's a new kwarg?

@ericl
Copy link
Contributor Author

ericl commented Mar 12, 2021

I believe this is only since it was adding a new kwarg. I think we should be able to avoid this kind of breakage in the future, but it might require some refactoring of the Ray client protocol (cc @AmeerHajAli )

@edoakes
Copy link
Contributor

edoakes commented Mar 12, 2021

@ericl @AmeerHajAli actually we could make the client API very defensive and just transparently pass through all args and kwargs where possible, then have the failure happen on the remote side. That might be the best way to avoid these issues coming up repeatedly.

Copy link
Contributor

@AmeerHajAli AmeerHajAli left a comment

Choose a reason for hiding this comment

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

Well... Should we add compatibility tests for ray client?

@ericl ericl merged commit b47036d into ray-project:master Mar 12, 2021
@ericl
Copy link
Contributor Author

ericl commented Mar 12, 2021

It's a good question, I think we need to decide whether to support x-version calls for nightly builds and so on. For properly released Ray versions, we require exact version match.

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.

None yet

5 participants