Skip to content

ci: Optimize docker layer and uv with no cache #1444

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.

Sign up for GitHub

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

Merged
thomasdhc merged 3 commits into from
Jan 30, 2026
Merged

ci: Optimize docker layer and uv with no cache #1444

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ca69252
Optimize docker layer and uv with no cache
thomasdhc Jan 30, 2026
79b1789
Add missing slash
thomasdhc Jan 30, 2026
301e14a
Add comments to dockerfile
thomasdhc Jan 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions docker/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ COPY docker/common/install_ffmpeg.sh .
RUN bash install_ffmpeg.sh && \
rm install_ffmpeg.sh

FROM nemo_curator_dep AS nemo_curator

ARG INTERN_VIDEO_COMMIT=09d872e5093296c6f36b8b3a91fc511b76433bf7
COPY external/intern_video2_multimodal.patch .
Expand All @@ -73,14 +72,19 @@ RUN git clone https://github.com/OpenGVLab/InternVideo.git && \
patch -p1 < /opt/intern_video2_multimodal.patch && \
rm /opt/intern_video2_multimodal.patch

FROM nemo_curator_dep AS nemo_curator

WORKDIR /opt/Curator

# Selective file copying before dependency installation for better layer caching
COPY pyproject.toml uv.lock /opt/Curator/
COPY nemo_curator/__init__.py nemo_curator/package_info.py /opt/Curator/nemo_curator/

@greptile-apps greptile-apps Bot Jan 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

copying __init__.py might cause issues during uv sync if setuptools tries to import it, since it imports cosmos_xenna.ray_utils.cluster which isn't installed yet

Suggested change
COPY nemo_curator/__init__.py nemo_curator/package_info.py /opt/Curator/nemo_curator/
COPY nemo_curator/package_info.py /opt/Curator/nemo_curator/


# Install Curator
RUN uv sync --link-mode copy --locked --extra all --all-groups --no-cache && \
uv add /opt/InternVideo/InternVideo2/multi_modality

Copilot AI Jan 30, 2026

Copy link

Choose a reason for hiding this comment

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

The uv add command on line 83 may create a cache even though --no-cache was used in the sync command. Consider adding --no-cache to the uv add command as well for consistency with the optimization goal of this PR.

Suggested change
uv add /opt/InternVideo/InternVideo2/multi_modality
uv add --no-cache /opt/InternVideo/InternVideo2/multi_modality

Copilot uses AI. Check for mistakes.

COPY . /opt/Curator
Comment on lines +80 to 87

@rlratzel rlratzel Jan 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not very familiar with uv, but I'm curious why we need to copy individual files from the repo on lines 78-79 if we just end up copying the entire repo on line 85? Can we just move line 85 up before the install and remove the individual file copy calls? I'm assuming 'no', but I'm curious why.

@thomasdhc thomasdhc Jan 30, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This optimizes docker re-build. If we copy over the entire Curator directory, every time Curator gets updated the cache will break leading to re-install of all dependencies. By isolating files needed for dependency vs changes to the code base, re-install does not need to happen if dependencies have not changed.

@rlratzel rlratzel Jan 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you for for explaining that. That's an excellent reason.

One minor request, feel free to ignore if you think it's obvious: would you mind also adding a comment to the Dockerfile mentioning that? I'm afraid a future maintainer might not pick up on that and refactor it out.

@thomasdhc thomasdhc Jan 30, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah let me add that note

RUN cd /opt/Curator && \
uv sync --link-mode copy --locked --extra all --all-groups && \
uv add /opt/InternVideo/InternVideo2/multi_modality && \
uv cache prune && \
uv cache clean distance && \
find /opt/uv_cache/ -type d -path "*ray/_private/runtime_env/agent/thirdparty_files/aiohttp*" -exec rm -rf {} +

COPY <<EOF /opt/venv/env.sh
export UV_PROJECT_ENVIRONMENT=/opt/venv
Expand Down
Loading