Skip to content

use container python interpreter instead of downloading a new one#41

Closed
chenb67 wants to merge 1 commit intoastral-sh:mainfrom
chenb67:patch-1
Closed

use container python interpreter instead of downloading a new one#41
chenb67 wants to merge 1 commit intoastral-sh:mainfrom
chenb67:patch-1

Conversation

@chenb67
Copy link
Copy Markdown

@chenb67 chenb67 commented Feb 2, 2025

otherwise the second stage breaks - the symlinks point to a non existing managed interpreter

otherwise the second stage breaks - the symlinks point to a non existing managed interpreter
@konstin konstin requested a review from zanieb February 2, 2025 16:09
@zanieb
Copy link
Copy Markdown
Member

zanieb commented Feb 2, 2025

Why would that be the case in this example? We explicitly use Python 3.12 bookworm-slim for both stages.

@chenb67
Copy link
Copy Markdown
Author

chenb67 commented Feb 2, 2025

the first stage uses uv to build the venv. the default config for uv is to manage the python interpreter which means it will download a fresh one instead of using the installed system interpreter
after copying the venv to the second stage without the managed interpreter, the venv python symbolic link is pointing to a non existent interpreter (the managed one from the first stage)

hope that makes it more clear

@zanieb
Copy link
Copy Markdown
Member

zanieb commented Feb 2, 2025

the first stage uses uv to build the venv. the default config for uv is to manage the python interpreter which means it will download a fresh one instead of using the installed system interprete

Except, by default, uv will not manage the Python interpreter if it can find one that satisfies your project's required version.

@moznuy
Copy link
Copy Markdown

moznuy commented Feb 2, 2025

I suggest adding to multistage example: (or some other check or warning)

COPY --from=builder --chown=app:app /app /app
# Test if python link is not broken.
# If this fails check that image python version satisfies your project requirement version
RUN test -e /app/.venv/bin/python

I spent a bit of time to figure out what's wrong with my simple test project before noticing that link was broken because image python version was too low. (this test fails in that scenario and works in good one)

@moznuy
Copy link
Copy Markdown

moznuy commented Feb 2, 2025

Or using --no-python-downloads (UV_PYTHON_DOWNLOADS=0) option with uv in the first stage - that also catches my issue.

@zanieb
Copy link
Copy Markdown
Member

zanieb commented Feb 3, 2025

Hm maybe... it's already called out explicitly

# It is important to use the image that matches the builder, as the path to the
# Python executable must be the same, e.g., using `python:3.11-slim-bookworm`
# will fail.

Disabling downloads seems fine too, but.. you can use downloads you just should copy the interpreter across to the second stage as well.

@chenb67
Copy link
Copy Markdown
Author

chenb67 commented Feb 3, 2025

the first stage uses uv to build the venv. the default config for uv is to manage the python interpreter which means it will download a fresh one instead of using the installed system interprete

Except, by default, uv will not manage the Python interpreter if it can find one that satisfies your project's required version.

for me even though the requires-python is set to >=3.12 uv still downloads a managed version. it's a workspace setup so it might be related (all the pyproject.toml files specify the same requires-python)

anyway in my opinion in this case(CI) it's better to be explicit about which interpreter to use instead of relying on implicit behaviour

@zanieb
Copy link
Copy Markdown
Member

zanieb commented Feb 3, 2025

Disabled downloads in #42

I think using UV_PYTHON is too confusing for the general example.

@zanieb
Copy link
Copy Markdown
Member

zanieb commented Feb 3, 2025

Similarly, I'll finish up #15 and merge that

@zanieb zanieb closed this Feb 9, 2025
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.

3 participants