Skip to content

Auto activate uv venv with pip installed #2666

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

r4victor
Copy link
Collaborator

@r4victor r4victor commented May 20, 2025

Addresses #2625

#2649 ditched conda in favor of uv in dstack Docker images. The side effect of that is that pip is no longer available in runs. This breaks things directly dependent on pip such as Jupyter's %pip, python -m venv, and pip hardcoded in makefiles and other scripts. The PR makes pip available by creating a uv venv with pip installed and activating it when using dstack Docker images. This should not worsen UX for users relying on uv-based workflow, only improve UX for those who depend on pip directly.

The PR will be merged once the 0.8 Docker images with uv are built and version.base_image is updated.

@r4victor r4victor requested a review from jvstme May 20, 2025 09:47
):
return []
return [
f"uv venv --prompt workflow --seed {DEFAULT_REPO_DIR}/.venv > /dev/null 2>&1",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I assume this .venv can turn up in git status, as it is not necessarily included in .gitignore. Any chance we could create it outside of the repo?

Copy link
Collaborator Author

@r4victor r4victor May 21, 2025

Choose a reason for hiding this comment

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

I considered creating a global uv venv outside the repo. The problem with that is that having an active venv outside the repo would trigger uv warnings about the active repo not being used cause it defaults to .venv in the project directory. Not critical, but quite confusing. And since we want to support uv-first flow, I think we should optimize for this case.

As regards to .venv in git, it will never appear in git since it's always excluded by .venv/.gitignore.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Users may still be confused by a new directory in their repository, especially if they don't use uv or Python. But I guess we can live with that if there are no good alternatives

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe, let's see how it works

@@ -164,6 +166,18 @@ async def _commands(self) -> List[str]:

return result

def _dstack_image_commands(self) -> List[str]:
if (
self.run_spec.configuration.image is not None
Copy link
Collaborator

Choose a reason for hiding this comment

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

(nit) This means pip won't work if our base image is explicitly specified in image. I've seen some users do it, possibly because we advertise it in the docs (1, 2, and more).

Maybe we could set up the venv in the Dockerfile, so that our images already come with a working pip?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The current idea is to have .venv in the repo dir so that uv does not issue warnings about the wrong venv. And I don't want to hardcode the repo path in the image since we had plans to make it configurable.

@r4victor
Copy link
Collaborator Author

Tested pip vs uv for installing pytorch on GCP n1-standard-4:

# time pip install torch
...
real    2m26.338s
user    1m37.514s
sys     0m16.711s
# time uv pip install torch
...
real    0m32.771s
user    0m29.070s
sys     0m8.300s

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.

2 participants