-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Moving Dockerfiles to uv for package installation #533
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -93,7 +93,7 @@ jobs: | |
|
|
||
| - name: Install dependencies | ||
| run: | | ||
| uv sync --dev | ||
| uv sync --group all --group dev | ||
| uv add pytest-cov | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Don’t mutate pyproject/lock in CI; declare pytest-cov in dev group instead.
Apply this diff to the workflow: - uv add pytest-cov
+ # pytest-cov should be declared in [dependency-groups.dev] in pyproject.toml
+ # and installed via the uv sync above.Then, in pyproject.toml (outside this file), ensure: [dependency-groups]
dev = [
"pytest",
"pytest-cov",
# ruff, mypy, etc.
]I can raise a follow-up PR to move pytest-cov (and ruff/mypy) into the dev group and drop this CI mutation. 🤖 Prompt for AI Agents |
||
|
|
||
| - name: Run linting with ruff (if available) | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,21 +1,24 @@ | ||||||||||
| # Server Service - Web crawling and document processing microservice | ||||||||||
|
|
||||||||||
| # Build stage | ||||||||||
| FROM python:3.11 AS builder | ||||||||||
| FROM python:3.12 AS builder | ||||||||||
|
|
||||||||||
| WORKDIR /build | ||||||||||
|
|
||||||||||
| # Install build dependencies | ||||||||||
| # Install build dependencies and uv | ||||||||||
| RUN apt-get update && apt-get install -y \ | ||||||||||
| build-essential \ | ||||||||||
| && rm -rf /var/lib/apt/lists/* | ||||||||||
| && rm -rf /var/lib/apt/lists/* \ | ||||||||||
| && pip install --no-cache-dir uv | ||||||||||
|
|
||||||||||
| # Copy pyproject.toml for dependency installation | ||||||||||
| COPY pyproject.toml . | ||||||||||
|
|
||||||||||
| # Copy and install Python dependencies | ||||||||||
| COPY requirements.server.txt . | ||||||||||
| RUN pip install --user --no-cache-dir -r requirements.server.txt | ||||||||||
| # Install server dependencies to a virtual environment using uv | ||||||||||
| RUN uv venv /venv && \ | ||||||||||
| . /venv/bin/activate && \ | ||||||||||
| uv pip install --group server | ||||||||||
|
|
||||||||||
| # Runtime stage | ||||||||||
| FROM python:3.11-slim | ||||||||||
| FROM python:3.12-slim | ||||||||||
|
|
||||||||||
| WORKDIR /app | ||||||||||
|
|
||||||||||
|
|
@@ -45,11 +48,11 @@ RUN apt-get update && apt-get install -y \ | |||||||||
| && apt-get clean \ | ||||||||||
| && rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/* | ||||||||||
|
|
||||||||||
| # Copy Python packages from builder | ||||||||||
| COPY --from=builder /root/.local /root/.local | ||||||||||
| # Copy the virtual environment from builder | ||||||||||
| COPY --from=builder /venv /venv | ||||||||||
|
|
||||||||||
| # Install Playwright browsers | ||||||||||
| ENV PATH=/root/.local/bin:$PATH | ||||||||||
| ENV PATH=/venv/bin:$PATH | ||||||||||
| RUN playwright install chromium | ||||||||||
|
Comment on lines
+55
to
56
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure Playwright is installed before invoking its CLI With the current pyproject, “playwright install chromium” will fail. After adding Playwright to the server group, prefer invoking via module for clarity. Apply: -ENV PATH=/venv/bin:$PATH
-RUN playwright install chromium
+ENV PATH=/venv/bin:$PATH
+RUN python -m playwright install chromium📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is working There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @coleam00 Thanks for confirming! You're right - since the virtual environment from the builder stage already contains Playwright (installed via the server group dependencies), it's available when the |
||||||||||
|
|
||||||||||
| # Copy server code and tests | ||||||||||
|
|
@@ -58,8 +61,9 @@ COPY src/__init__.py src/ | |||||||||
| COPY tests/ tests/ | ||||||||||
|
|
||||||||||
| # Set environment variables | ||||||||||
| ENV PYTHONPATH="/app:/root/.local/lib/python3.11/site-packages:$PYTHONPATH" | ||||||||||
| ENV PYTHONPATH="/app:$PYTHONPATH" | ||||||||||
| ENV PYTHONUNBUFFERED=1 | ||||||||||
| ENV PATH="/venv/bin:$PATH" | ||||||||||
|
|
||||||||||
| # Expose Server port | ||||||||||
| ARG ARCHON_SERVER_PORT=8181 | ||||||||||
|
|
||||||||||
This file was deleted.
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Make installs deterministic: add --locked to uv sync.
Without --locked, uv may relock/modify during CI. Using --locked fails fast if the lockfile is out-of-date, which is what we want in CI. Also, dev is synced by default, but keeping it explicit is fine. (docs.astral.sh)
Apply this diff:
📝 Committable suggestion
🤖 Prompt for AI Agents