-
Notifications
You must be signed in to change notification settings - Fork 531
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
[Core] Disable conda activate for custom docker image as runtime environment #3874
Conversation
@@ -126,7 +126,7 @@ | |||
# https://github.com/ray-project/ray/issues/31606 | |||
# We use python 3.10 to be consistent with the python version of the | |||
# AWS's Deep Learning AMI's default conda environment. | |||
CONDA_INSTALLATION_COMMANDS = ( | |||
CONDA_INSTALLATION_COMMANDS = lambda conda_auto_activate: ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Can we just use .format
for this purpose? Having a lambda function in the constants.py
is a bit weird.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually tried, but .format
does not work for those {}
in the installation commands..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, in that case, let's have a comment here for why we use lambda
function here. : )
Smoke test for this also passed! |
Blocked by #3867 and fixes #3741.
Tested (run the relevant ones):
bash format.sh
pytest tests/test_smoke.py
pytest tests/test_smoke.py::test_docker_storage_mounts
pytest tests/test_smoke.py::test_job_queue_with_docker
pytest tests/test_smoke.py::test_docker_preinstalled_package
conda deactivate; bash -i tests/backward_compatibility_tests.sh