[4/6 Deps Update] Update docker images use uv#399
Conversation
| wget \ | ||
| cmake \ | ||
| iputils-ping \ | ||
| curl \ |
There was a problem hiding this comment.
qq why do we need curl here?
There was a problem hiding this comment.
Used for downloading uv installer
GiGL/requirements/install_py_deps.sh
Line 62 in dbc161f
There was a problem hiding this comment.
hmmm, this also means any dev boxes we have also need curl right? I think they have it by default but maybe they don't? Should the curl instatllation be elsewhere?
There was a problem hiding this comment.
Should the curl instatllation be elsewhere?
Like in install_deps?
I feel like its a pretty common package id rather leave it where it is for now and not increate. the scope even more trying to centralize ubuntu package installations to some shared script.
I do believe it comes preinstalled in the dev boxes, and even if it doesnt I can argue for something like that maybe we should not be installing it for the user, similar to how we dont install docker, etc.
|
|
||
|
|
||
| # Dec 1, 2025 (svij-sc): | ||
| # GCP Cloud build agents run an older version of docker deamon |
There was a problem hiding this comment.
will we need to update docker version when gcp packages update? As we don't fix the gcp package versions and they auto resolve
There was a problem hiding this comment.
this issue is diff than GCP packages.
This is more an issue w. docker deamon running on the GCP cloud build agent VMs* that run our CI/CD.
containers/Dockerfile.builder
Outdated
| ENV PATH="${UV_PROJECT_ENVIRONMENT}/bin:${PATH}" | ||
| # We also need to make UV detectable by the system | ||
| ENV PATH="/root/.local/bin:${PATH}" | ||
| RUN uv tool install pip==25.3 |
There was a problem hiding this comment.
Do we need to fix pip version?
There was a problem hiding this comment.
More so, why don't we lock the pip version we install with uv?
There was a problem hiding this comment.
Good catch, I locked it in pyproject.toml; let me remove this from here and see what happens
containers/Dockerfile.builder
Outdated
| ENV PATH="${UV_PROJECT_ENVIRONMENT}/bin:${PATH}" | ||
| # We also need to make UV detectable by the system | ||
| ENV PATH="/root/.local/bin:${PATH}" | ||
| RUN uv tool install pip==25.3 |
There was a problem hiding this comment.
More so, why don't we lock the pip version we install with uv?
| wget \ | ||
| cmake \ | ||
| iputils-ping \ | ||
| curl \ |
There was a problem hiding this comment.
hmmm, this also means any dev boxes we have also need curl right? I think they have it by default but maybe they don't? Should the curl instatllation be elsewhere?
| # Already has python 3.11 installed - no need to install it again. | ||
| # We use system python since it has packages pre-installed for us. | ||
| ENV UV_SYSTEM_PYTHON=true | ||
| ENV UV_PROJECT_ENVIRONMENT=/opt/conda/ |
There was a problem hiding this comment.
Again I'm not sure we should be using system python (and we generally want to avoid conda?)
I assume we set this to use /opt/conda because the default torch images use that one?
There was a problem hiding this comment.
I assume we set this to use /opt/conda because the default torch images use that one?
This is exactly it. Their python installation is using conda-forge
Re:
Again I'm not sure we should be using system python
Lets continue our conversation in the other thread https://github.com/Snapchat/GiGL/pull/398/files#r2565901566
There was a problem hiding this comment.
Unrelated but do you think it's a bit confusing for us to have different python install dirs for different images?
There was a problem hiding this comment.
note I also don't think we should be using /opt/conda if we are in fact no using conda - I guess it's fine for us to have the different install paths.
There was a problem hiding this comment.
This is a prod docker image not a dev container.
If we dont use the default system python we need to then reinstall everything into some other environment and we lose on the benifits of having cuda libs/drivers/pytorch pre-installed installed for us.
Especially because we are not using conda to manage the rest of the deps.
There was a problem hiding this comment.
Secondly, this also causes issues listed here: #398 (comment)
| # Already has python 3.11 installed - no need to install it again. | ||
| # We use system python since it has packages pre-installed for us. | ||
| ENV UV_SYSTEM_PYTHON=true | ||
| ENV UV_PROJECT_ENVIRONMENT=/opt/conda/ |
There was a problem hiding this comment.
Unrelated but do you think it's a bit confusing for us to have different python install dirs for different images?
| # Already has python 3.11 installed - no need to install it again. | ||
| # We use system python since it has packages pre-installed for us. | ||
| ENV UV_SYSTEM_PYTHON=true | ||
| ENV UV_PROJECT_ENVIRONMENT=/opt/conda/ |
There was a problem hiding this comment.
note I also don't think we should be using /opt/conda if we are in fact no using conda - I guess it's fine for us to have the different install paths.
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Background Context: #405
Some guidance on stack of PRs: #405 (comment)
Scope of PR
COPY --from=apache/beam_python3.9_sdk:2.53.0 /opt/apache/beam /opt/apache/beamwith the updated python and sdk version creates runtime failures when running dataflow jobs. Thus, we are now just usingapache/beam_python3.11_sdk:2.56.0as base image instead of trying to copy from it.