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

[Core] Avoid install python 3.11 conda #2241

Merged
merged 8 commits into from
Jul 16, 2023
Merged

Conversation

Michaelvll
Copy link
Collaborator

@Michaelvll Michaelvll commented Jul 14, 2023

Partially fixes #2240, for the images without conda installed by default.

This PR also updates the tf version for tpu tests as the TPU no longer support the old version of Tensorflow https://cloud.google.com/tpu/docs/supported-tpu-configurations#tensorflow_2

Confirmed that this PR fixes the TPU VM issue, as the original image of the TPU VM does not have conda installed by default.

Tested (run the relevant ones):

  • Code formatting: bash format.sh
  • All smoke tests: pytest tests/test_smoke.py
  • All smoke tests: pytest tests/test_smoke.py --aws

@Michaelvll Michaelvll changed the title Avoid install python 3.11 conda [Core] Avoid install python 3.11 conda Jul 14, 2023
Copy link
Member

@concretevitamin concretevitamin left a comment

Choose a reason for hiding this comment

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

Thanks for identifying the issue @Michaelvll! Left a question about skipping some smoke tests.

sky/skylet/constants.py Show resolved Hide resolved
@@ -1506,20 +1509,26 @@ def _get_cancel_task_with_cloud(name, cloud, timeout=15 * 60):

# ---------- Testing `sky cancel` ----------
@pytest.mark.aws
@pytest.mark.skip(
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, it seems a bit dangerous to skip these core tests? Maybe we should endure the flakiness for now?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Should we instead fix #2207 ? It seems to be problematic tests.

Copy link
Member

Choose a reason for hiding this comment

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

should we mark it as P0? To avoid skipping these core tests for too long.

@@ -190,7 +190,7 @@ setup_commands:
pip3 --version > /dev/null 2>&1 || (curl -sSL https://bootstrap.pypa.io/get-pip.py -o get-pip.py && python3 get-pip.py && echo "PATH=$HOME/.local/bin:$PATH" >> ~/.bashrc);
(type -a python | grep -q python3) || echo 'alias python=python3' >> ~/.bashrc;
(type -a pip | grep -q pip3) || echo 'alias pip=pip3' >> ~/.bashrc;
(which conda > /dev/null 2>&1 && conda init > /dev/null) || (wget -nc https://repo.anaconda.com/miniconda/Miniconda3-latest-Linux-x86_64.sh && bash Miniconda3-latest-Linux-x86_64.sh -b && eval "$(~/miniconda3/bin/conda shell.bash hook)" && conda init && conda config --set auto_activate_base true);
Copy link
Member

Choose a reason for hiding this comment

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

These couple of j2 seem to have 2 - 3 versions. (e.g., some have conda init > /dev/null, some don't). I guess if smoke tests all passed (does it by default cover all tests?) it's ok to use 1 version?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It should be fine. The only difference between the two versions is just having conda init after the which conda at the beginning. Theoretically, it should not affect anything. We now have the conda init everywhere, and tested with pytest tests/test_smoke.py (on GCP) which indicates adding the conda init indeed does not cause problem.

@Michaelvll Michaelvll merged commit 95fdb5b into master Jul 16, 2023
@Michaelvll Michaelvll deleted the avoid-installing-python311 branch July 16, 2023 02:05
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.

[Python 3.11] All remote image with python 3.11 is not supported
2 participants