Skip to content

Conversation

@amc-corey-cox
Copy link
Collaborator

While I was doing some documentation improvement, Claude identified that Docker is using poetry when we have now switched to uv. This looked like an easy fix so I've created a PR for it here. If you don't like how this changes things feel free to throw it out.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR migrates the Docker build configuration from Poetry to uv, aligning the containerized environment with the project's package manager transition. The changes remove Poetry-specific configurations and replace them with uv equivalents.

Key Changes:

  • Replaced Poetry installation with uv using the official install script
  • Updated dependency installation from poetry install --with env --with dev to uv sync --frozen
  • Updated the container's default command from poetry run dm-bip run to uv run dm-bip run

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
Dockerfile Migrated from Poetry to uv: removed Poetry environment variables and installation, added curl dependency, installed uv via official script, updated dependency sync and run commands
DOCKER.md Updated documentation to reflect the new default command using uv run instead of poetry run

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +14 to +15
RUN curl -LsSf https://astral.sh/uv/install.sh | sh
ENV PATH="/root/.local/bin:$PATH"
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

Consider using the official uv Docker image as a base or installing uv via pip for better reproducibility in Docker environments. The shell script installation method may not pin to a specific version and could lead to inconsistent builds over time.

Recommended alternatives:

  • Use COPY --from=ghcr.io/astral-sh/uv:latest /uv /usr/local/bin/uv to copy the binary from the official image
  • Or use pip install uv with a pinned version for reproducibility
Suggested change
RUN curl -LsSf https://astral.sh/uv/install.sh | sh
ENV PATH="/root/.local/bin:$PATH"
COPY --from=ghcr.io/astral-sh/uv:latest /uv /usr/local/bin/uv

Copilot uses AI. Check for mistakes.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@vbakalov can you give an opinion on this one and we'll decide how to resolve it?

Comment on lines 19 to +23
# Copy the entire project
COPY . .

# Install dependencies using the same groups as on your local machine
# Uses the lock file; will run inside Poetry's own virtualenv
RUN poetry install --with env --with dev


# Install dependencies using uv
RUN uv sync --frozen
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider optimizing Docker layer caching by copying only the dependency files before copying the entire project. This allows Docker to cache the dependency installation layer and only rebuild it when dependencies change, not when code changes.

Recommended structure:

# Copy dependency files first for caching
COPY pyproject.toml uv.lock ./

# Install dependencies using uv
RUN uv sync --frozen

# Copy the rest of the project
COPY . .

This can significantly speed up Docker builds during development when only code changes, not dependencies.

Copilot uses AI. Check for mistakes.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This sounds good but I'm not sure I understand it. We can ignore it if this seems like a lot of work, especially since things are a little fluid now. @vbakalov let me know what you think.

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