[CI] [Minimal install] Check python version in minimal install #36887
Conversation
Signed-off-by: Cade Daniel <cade@anyscale.com>
Signed-off-by: Cade Daniel <cade@anyscale.com>
32e953e to
4211441
Compare
Signed-off-by: Cade Daniel <cade@anyscale.com>
Signed-off-by: Cade Daniel <cade@anyscale.com>
| BAZEL_EXPORT_OPTIONS="$(./ci/run/bazel_export_options)" | ||
| # Ignoring shellcheck is necessary because if ${BAZEL_EXPORT_OPTIONS} is wrapped by the double quotation, | ||
| # bazel test cannot recognize the option. | ||
| # shellcheck disable=SC2086 | ||
| bazel test --test_output=streamed --config=ci --test_env=RAY_MINIMAL=1 --test_env=EXPECTED_PYTHON_VERSION=$EXPECTED_PYTHON_VERSION ${BAZEL_EXPORT_OPTIONS} python/ray/tests/test_minimal_install |
There was a problem hiding this comment.
I was thinking of an "easier" approach where we just asserted the python version in shell here like check the python --version output.
What's the motif for running another pytest and assert in there?
There was a problem hiding this comment.
I'm not sure how bazel works, so wanted to verify that bazel picks up right version. If someone confident in bazel behavior says we can trust it, I can delete the test.
There was a problem hiding this comment.
I don't have a strong opinion here - I' m ok with the current approach.
|
Are you saying when we ask conda to install a particular version of python and that version doesn't exist, conda won't fail loudly but silently install another version? |
|
Maybe not conda not fail explicitly - but our CI failed silently. |
Should we fix this directly? It seems to me that we still don't know the exact root cause yet? |
I don't think we know what the root cause was. If we have repro steps or logs I can go investigate. This PR is to fail loudly the next time it happens so we can look into it. |
Yeah - i think the root cause was not really known, all we know now is that it wasn't running the expected python version somehow. |
|
edit: fixed |
Signed-off-by: Cade Daniel <cade@anyscale.com>
|
cc @jjyao ready for merge |
I merged this but can we figure out the root cause as well (not a high priority) |
…roject#36887) This PR improves our CI by asserting the expected Python version in our minimal install tests. We use these tests to validate basic Ray functionality for each Python version we support (3.7-3.11). In the past, we saw an issue where the 3.11 tests actually used a different Python version because the conda env we consumed hadn't yet been built for 3.11. Now, the tests will fail if the actual Python version differs from the expected one. Signed-off-by: Cade Daniel <cade@anyscale.com> Signed-off-by: e428265 <arvind.chandramouli@lmco.com>
This PR improves our CI by asserting the expected Python version in our minimal install tests. We use these tests to validate basic Ray functionality for each Python version we support (3.7-3.11). In the past, we saw an issue where the 3.11 tests actually used a different Python version because the conda env we consumed hadn't yet been built for 3.11. Now, the tests will fail if the actual Python version differs from the expected one.
See also #33401