[runtime env] Support python 3.10 for runtime_env conda #30970

architkulkarni · 2022-12-08T18:17:17Z

Signed-off-by: Archit Kulkarni architkulkarni@users.noreply.github.com

Why are these changes needed?

conda environments are isolated, so when runtime_env sets up a conda environment it must download the Ray wheel into the conda environment. It must download the wheel that matches the current Python and Ray version running, otherwise there will be incompatibility issues between the workers that use this runtime_env and the other workers and Ray processes.

This PR updates the wheel name format logic to support Python 3.10.

Related issue number

Closes #30929

Checks

I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
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 :(

Signed-off-by: Archit Kulkarni <architkulkarni@users.noreply.github.com>

architkulkarni · 2022-12-08T18:19:07Z

@scv119 If there is a "release process" for new Python versions, could this PR be linked there and added as a step?

We could make this more futureproof by removing the assert py_version in ["36", "37", "38", "39", ...], but it would just delay the eventual breakage whenever the wheel name format changes, and we'd still need to manually update the tests.

edoakes

@architkulkarni is there a way we can fail very loudly when Ray runs a new python version but it isn't supported in this enum? At a minimum we can add a small CI test (just check current py version vs. this map)

edoakes · 2022-12-08T19:12:19Z

Or maybe the existing test is good enough... just checking we catch this as early as possible

Signed-off-by: Archit Kulkarni <architkulkarni@users.noreply.github.com>

architkulkarni · 2022-12-08T21:34:48Z

@edoakes Good point, the existing tests will just pass even if a new version is added.

I added a constant SUPPORTED_PY_VERSIONS and a unit test to check the current py version against it to make sure the constant stays updated.

Unfortunately, there's still no way to auto-update the test_get_*_wheel CI tests because of a chicken and egg situation. The test needs wheels to be uploaded to AWS in the standard location, but the wheels get uploaded there only after the PR is merged. It's probably not too bad to have a manual process here because new python versions only come once a year.

architkulkarni · 2022-12-08T21:35:20Z

python/ray/_private/utils.py

+    elif py_version_str in ["38", "39"]:
+        darwin_os_string = "macosx_10_15_x86_64"
+    else:
+        darwin_os_string = "macosx_10_15_universal2"


New OS string just dropped.

Signed-off-by: Archit Kulkarni <architkulkarni@users.noreply.github.com>

architkulkarni · 2022-12-10T01:02:03Z

test_huggingface was broken on master, unrelated
rllbi test_memory_leak was flaky on master, unrelated

architkulkarni · 2022-12-13T18:24:45Z

@scv119 @edoakes Any other comments on this PR?

rkooo567

minor comments

rkooo567 · 2022-12-16T15:05:09Z

python/ray/_private/utils.py

@@ -1304,7 +1308,7 @@ def get_release_wheel_url(
    ray_commit: str = ray.__commit__,
    sys_platform: str = sys.platform,
    ray_version: str = ray.__version__,
-    py_version: str = f"{sys.version_info.major}{sys.version_info.minor}",
+    py_version: Tuple[int, int] = sys.version_info[:2],


why do we fix this? The previous code seems to be easier to understand

No strong reason, I just think it's better not to convert it to a string until the very end of the data flow, when we actually generate the wheel name.

But I agree that sys.version_info[:2] is harder to understand. I'll change it to (sys.version_info.major, sys.version_info.minor)

rkooo567 · 2022-12-16T15:05:20Z

python/ray/tests/test_runtime_env.py

    # NOTE: These should not be changed for releases.
    ray_version = "3.0.0.dev0"
    for sys_platform in ["darwin", "linux", "win32"]:
-        for py_version in ["36", "37", "38", "39"]:
-            if sys_platform == "win32" and py_version == "36":
+        for py_version in [(3, 6), (3, 7), (3, 8), (3, 9), (3, 10)]:


Use the same constant?

Signed-off-by: Archit Kulkarni <architkulkarni@users.noreply.github.com>

This PR updates the release tests wheel_urls to make it compatible with the internal API change in PR #30970. Previously the release test was breaking with File "/home/ray/anaconda3/lib/python3.7/site-packages/ray/_private/utils.py", line 1266, in get_wheel_filename assert py_version in ray_constants.RUNTIME_ENV_CONDA_PY_VERSIONS, py_version AssertionError: 36

Signed-off-by: Archit Kulkarni <architkulkarni@users.noreply.github.com> conda environments are isolated, so when runtime_env sets up a conda environment it must download the Ray wheel into the conda environment. It must download the wheel that matches the current Python and Ray version running, otherwise there will be incompatibility issues between the workers that use this runtime_env and the other workers and Ray processes. This PR updates the wheel name format logic to support Python 3.10.

This PR updates the release tests wheel_urls to make it compatible with the internal API change in PR #30970. Previously the release test was breaking with File "/home/ray/anaconda3/lib/python3.7/site-packages/ray/_private/utils.py", line 1266, in get_wheel_filename assert py_version in ray_constants.RUNTIME_ENV_CONDA_PY_VERSIONS, py_version AssertionError: 36

…30970) Signed-off-by: Archit Kulkarni <architkulkarni@users.noreply.github.com> conda environments are isolated, so when runtime_env sets up a conda environment it must download the Ray wheel into the conda environment. It must download the wheel that matches the current Python and Ray version running, otherwise there will be incompatibility issues between the workers that use this runtime_env and the other workers and Ray processes. This PR updates the wheel name format logic to support Python 3.10. Signed-off-by: tmynn <hovhannes.tamoyan@gmail.com>

This PR updates the release tests wheel_urls to make it compatible with the internal API change in PR ray-project#30970. Previously the release test was breaking with File "/home/ray/anaconda3/lib/python3.7/site-packages/ray/_private/utils.py", line 1266, in get_wheel_filename assert py_version in ray_constants.RUNTIME_ENV_CONDA_PY_VERSIONS, py_version AssertionError: 36 Signed-off-by: tmynn <hovhannes.tamoyan@gmail.com>

architkulkarni added 2 commits December 8, 2022 10:11

Support python 3.10 for runtime_env conda

5cdc9b0

Signed-off-by: Archit Kulkarni <architkulkarni@users.noreply.github.com>

Futureproof x86_64 if condition for Mac

d89e66e

Signed-off-by: Archit Kulkarni <architkulkarni@users.noreply.github.com>

architkulkarni assigned scv119, edoakes and rkooo567 Dec 8, 2022

edoakes reviewed Dec 8, 2022

View reviewed changes

Add SUPPORTED_PY_VERSIONS and unit test

1150c5d

Signed-off-by: Archit Kulkarni <architkulkarni@users.noreply.github.com>

architkulkarni commented Dec 8, 2022

View reviewed changes

Fix typo

0bacfc4

Signed-off-by: Archit Kulkarni <architkulkarni@users.noreply.github.com>

architkulkarni added the tests-ok The tagger certifies test failures are unrelated and assumes personal liability. label Dec 10, 2022

rkooo567 approved these changes Dec 16, 2022

View reviewed changes

rkooo567 added the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Dec 16, 2022

architkulkarni added 2 commits December 16, 2022 11:04

Make version info more readable

ac4870e

Signed-off-by: Archit Kulkarni <architkulkarni@users.noreply.github.com>

Use constant in test

cc9f630

Signed-off-by: Archit Kulkarni <architkulkarni@users.noreply.github.com>

rkooo567 merged commit 98fef77 into ray-project:master Dec 19, 2022

architkulkarni mentioned this pull request Dec 20, 2022

[runtime_env] Fix failing wheel_urls release test #31239

Merged

7 tasks

rickyyx mentioned this pull request Dec 23, 2022

[core][nightly] many_actors regression #31284

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[runtime env] Support python 3.10 for runtime_env conda #30970

[runtime env] Support python 3.10 for runtime_env conda #30970

architkulkarni commented Dec 8, 2022

architkulkarni commented Dec 8, 2022

edoakes left a comment •

edited

Loading

edoakes commented Dec 8, 2022

architkulkarni commented Dec 8, 2022

architkulkarni Dec 8, 2022

edoakes Dec 8, 2022

architkulkarni commented Dec 10, 2022

architkulkarni commented Dec 13, 2022

rkooo567 left a comment

rkooo567 Dec 16, 2022

architkulkarni Dec 16, 2022

rkooo567 Dec 16, 2022

[runtime env] Support python 3.10 for runtime_env conda #30970

[runtime env] Support python 3.10 for runtime_env conda #30970

Conversation

architkulkarni commented Dec 8, 2022

Why are these changes needed?

Related issue number

Checks

architkulkarni commented Dec 8, 2022

edoakes left a comment • edited Loading

Choose a reason for hiding this comment

edoakes commented Dec 8, 2022

architkulkarni commented Dec 8, 2022

architkulkarni Dec 8, 2022

Choose a reason for hiding this comment

edoakes Dec 8, 2022

Choose a reason for hiding this comment

architkulkarni commented Dec 10, 2022

architkulkarni commented Dec 13, 2022

rkooo567 left a comment

Choose a reason for hiding this comment

rkooo567 Dec 16, 2022

Choose a reason for hiding this comment

architkulkarni Dec 16, 2022

Choose a reason for hiding this comment

rkooo567 Dec 16, 2022

Choose a reason for hiding this comment

edoakes left a comment •

edited

Loading