-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Use UV to manage python dependencies #8404
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
base: preview
Are you sure you want to change the base?
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughConsolidates API Docker build into a single multi-stage Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-11-25T10:17:39.709ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
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.
Actionable comments posted: 4
🧹 Nitpick comments (3)
apps/api/.dockerignore (1)
1-3: Consider adding common Python artifacts to .dockerignore.While the current exclusions are correct, adding more patterns would optimize the Docker build context size and prevent unnecessary files from being copied during builds.
🔎 Suggested additional exclusions
venv/ .venv/ Dockerfile +__pycache__/ +*.py[cod] +*$py.class +.pytest_cache/ +.coverage +htmlcov/ +.git/ +.gitignore +*.log +.env +.env.local +*.mdapps/api/Dockerfile (2)
1-26: Consider pinning the UV version for reproducibility.The builder stage uses
ghcr.io/astral-sh/uv:latestwhich may lead to non-reproducible builds if UV releases breaking changes. The rest of the builder stage follows best practices with cache mounts and two-phase dependency installation.🔎 Suggested fix to pin UV version
# Stage 1: Builder - Install dependencies with uv FROM python:3.12.10-alpine AS builder # Install uv -COPY --from=ghcr.io/astral-sh/uv:latest /uv /usr/local/bin/uv +COPY --from=ghcr.io/astral-sh/uv:0.5 /uv /usr/local/bin/uvCheck the latest stable UV version and pin accordingly.
51-51: Consider if bash is necessary in production.The production stage installs bash, which increases the image size and attack surface. If the entrypoint scripts can be rewritten to use sh (which is already in Alpine), you could remove this dependency.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/api/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
.github/workflows/build-branch.yml(1 hunks)apps/api/.dockerignore(1 hunks)apps/api/Dockerfile(1 hunks)apps/api/Dockerfile.api(0 hunks)apps/api/Dockerfile.dev(0 hunks)apps/api/pyproject.toml(2 hunks)apps/api/requirements.txt(0 hunks)apps/api/requirements/base.txt(0 hunks)apps/api/requirements/local.txt(0 hunks)apps/api/requirements/production.txt(0 hunks)apps/api/requirements/test.txt(0 hunks)deployments/cli/community/build.yml(1 hunks)docker-compose.yml(4 hunks)
💤 Files with no reviewable changes (7)
- apps/api/requirements/production.txt
- apps/api/Dockerfile.dev
- apps/api/requirements/test.txt
- apps/api/Dockerfile.api
- apps/api/requirements/base.txt
- apps/api/requirements/local.txt
- apps/api/requirements.txt
🧰 Additional context used
📓 Path-based instructions (1)
docker-compose.yml
📄 CodeRabbit inference engine (.github/instructions/bash.instructions.md)
Use docker-compose.yml for production and staging Docker configuration
Files:
docker-compose.yml
🧠 Learnings (2)
📚 Learning: 2025-11-25T10:17:39.709Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/bash.instructions.md:0-0
Timestamp: 2025-11-25T10:17:39.709Z
Learning: Applies to docker-compose.yml : Use docker-compose.yml for production and staging Docker configuration
Applied to files:
docker-compose.ymldeployments/cli/community/build.yml
📚 Learning: 2025-11-25T10:17:39.709Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/bash.instructions.md:0-0
Timestamp: 2025-11-25T10:17:39.709Z
Learning: Applies to docker-compose-local.yml : Use docker-compose-local.yml for local development Docker configuration
Applied to files:
docker-compose.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (6)
apps/api/pyproject.toml (3)
1-5: LGTM! Verify Python 3.12 availability in deployment environments.The project metadata updates are clear and appropriate. The name, version, and description accurately reflect the component's purpose.
Ensure that all deployment targets (Docker base images, CI/CD environments, production servers) support Python 3.12 or higher.
6-48: Address beta version in production dependencies and review psycopg package redundancy.The opentelemetry-instrumentation-django==0.49b1 is a beta release, which should not be pinned in production dependencies. Consider upgrading to a stable version.
Additionally, the dependency list includes three psycopg packages (psycopg==3.3.0, psycopg-binary==3.3.0, psycopg-c==3.3.0). For Psycopg 3, the binary files are separated, so you need to install psycopg itself along with optional components, but installing all three together is unusual. Typically you would install either
psycopg[binary]orpsycopg[c]as an extra, not both as separate packages.Regarding strict == pinning: While it prevents automatic security updates, this is a valid reproducibility strategy and requires active maintenance to address CVEs rather than being inherently problematic.
142-158: Dependency groups are well-organized.The separation of
localandtestdependencies is appropriate. Development tools likedjango-debug-toolbarandruffare correctly isolated to thelocalgroup, and test utilities are properly grouped. Therequests==2.32.4dependency in the test group does not appear in the main project dependencies, so there are no version conflicts to resolve.docker-compose.yml (1)
41-41: LGTM! Consistent Dockerfile path updates.The dockerfile references have been correctly updated from
Dockerfile.apitoDockerfileacross all API-related services (api, worker, beat-worker, migrator). The build context remains unchanged at./apps/api, ensuring consistency.As per coding guidelines, this docker-compose.yml is appropriately used for production and staging Docker configuration.
Also applies to: 56-56, 72-72, 88-88
.github/workflows/build-branch.yml (1)
243-243: LGTM! Dockerfile path correctly updated.The dockerfile-path has been properly updated to reference the new unified
./apps/api/Dockerfile, consistent with the dockerfile consolidation across the repository.deployments/cli/community/build.yml (1)
30-30: LGTM! Dockerfile path updated consistently.The dockerfile reference has been correctly updated to
Dockerfile, consistent with the dockerfile consolidation effort across all deployment configurations.Based on learnings, this build.yml file is appropriately used for community deployment builds.
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.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/api/Dockerfile (1)
28-35: Fix the CMD path in the dev stage.The project is copied to
./plane(Line 31), but the CMD references./bin/docker-entrypoint-api-local.shwhich resolves to/app/bin/. The entrypoint is at/app/plane/bin/docker-entrypoint-api-local.sh. Either change the CMD path or add a WORKDIR directive like the production stage.🔎 Proposed fix
# Optional dev environment FROM builder AS dev # Copy application source code COPY . ./plane RUN --mount=type=cache,target=/root/.cache/uv \ uv sync --locked --group local -CMD [ "./bin/docker-entrypoint-api-local.sh" ] +WORKDIR /app/plane +CMD ["./bin/docker-entrypoint-api-local.sh"]
🧹 Nitpick comments (1)
apps/api/Dockerfile (1)
51-51: Add--no-cacheto apk command.Using
--no-cacheavoids storing the package index locally, reducing image size.🔎 Proposed fix
-RUN apk add bash +RUN apk add --no-cache bash
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/api/Dockerfile(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (2)
apps/api/Dockerfile (2)
38-43: Test stage is missing a CMD or entrypoint.The test stage syncs test dependencies but doesn't define a CMD. If tests are run via explicit
docker runcommands this may be intentional, but consider adding a default command for consistency.Is the test stage intended to be run with an explicit command override (e.g.,
docker run <image> pytest)? If so, this is fine. Otherwise, consider adding a default CMD.
46-70: Production stage looks correct.The production stage properly sets
WORKDIR /app/planebefore referencing./bin/docker-entrypoint-api.sh, ensuring the entrypoint script is found at the correct path. The virtual environment is correctly copied and added to PATH.
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.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/api/pyproject.toml (3)
23-23: Consider using a version range instead of exact pin for Faker.The
fakerpackage is pinned to an exact version (==25.0.0), which is unusual for a production dependency. Exact pins make it harder to receive patch updates and security fixes. Consider using a range constraint like>=25.0.0,<26.0unless there's a specific reason for the exact pin.
35-35: Consider using extras notation for psycopg binary package.The dependency uses
psycopg-binaryas a standalone package. While this works, the official Psycopg 3 documentation recommends using extras notation:psycopg[binary]orpsycopg[c]instead of separate packages. This is the modern approach and provides better clarity about the optimization being used.🔎 Suggested change
- "psycopg-binary>=3.3.0,<4.0", + "psycopg[binary]>=3.3.0,<4.0",
145-156: Consider updating test dependencies to more recent versions.Several test dependencies are outdated and should be refreshed:
pytest==7.4.0(released July 2023)coverage==7.2.7(released 2023)httpx==0.24.1(released June 2023)Updating to current versions will provide access to recent bug fixes, performance improvements, and new features.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/api/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
apps/api/pyproject.toml(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (1)
apps/api/pyproject.toml (1)
5-5: Consider relaxing the Python 3.12 minimum requirement.The
requires-python = ">=3.12"constraint is unnecessarily strict. Django 4.2 supports Python 3.8, 3.9, 3.10, 3.11, and 3.12, and none of the listed dependencies require Python 3.12+. Relaxing this to>=3.11or>=3.10would maintain compatibility without limiting deployments to only the most recent Python version.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/api/Dockerfile (2)
29-38: Align dev stage WORKDIR with the copied code location.The dev stage copies the project to
./plane(/app/plane) but continues executing from/app(the builder's copy). This means theCOPY . ./planeat line 32 is unused, and commands reference the builder's code at/appinstead. The production stage correctly handles this by settingWORKDIR /app/planeafter copying (line 78).🔎 Suggested alignment with production pattern
FROM builder AS dev # Copy application source code COPY . ./plane RUN --mount=type=cache,target=/root/.cache/uv \ uv sync --locked --group local RUN apk add bash +WORKDIR /app/plane CMD [ "./bin/docker-entrypoint-api-local.sh" ]Alternatively, if the builder's copy at
/appshould be used directly, remove the redundantCOPY . ./planestep.
41-48: Test stage has the same WORKDIR pattern issue as dev.Similar to the dev stage, the test stage copies to
./planebut doesn't changeWORKDIR, so the copied code remains unused. Additionally, noCMDis specified—if this is intentional for runtime override, that's fine; otherwise consider adding one.🔎 Suggested consistency with production
FROM builder AS test # Copy application source code COPY . ./plane RUN --mount=type=cache,target=/root/.cache/uv \ uv sync --locked --group test RUN apk add bash + +WORKDIR /app/plane +# Optionally add: CMD [ "./bin/docker-entrypoint-api-test.sh" ]
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/api/Dockerfile(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-25T10:17:39.709Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/bash.instructions.md:0-0
Timestamp: 2025-11-25T10:17:39.709Z
Learning: Applies to docker-compose.yml : Use docker-compose.yml for production and staging Docker configuration
Applied to files:
apps/api/Dockerfile
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (2)
apps/api/Dockerfile (2)
1-27: Builder stage structure looks solid.The two-phase
uv syncpattern (lines 15-18 without project, lines 25-26 with project) is idiomatic for Docker layer caching, and the uv version is appropriately pinned per previous feedback.
50-80: Production entrypoint correctly uses Gunicorn with proper configuration.The
./bin/docker-entrypoint-api.shscript uses Gunicorn with uvicorn workers and configurable worker management (viaGUNICORN_WORKERSenvironment variable), along with sensible production defaults like request recycling limits and jitter. The production stage is properly configured for deployment.
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/api/Dockerfile (2)
40-42: Build dependencies are removed before dev/test stages inherit.The
apk del .build-depsat line 42 removes gcc, musl-dev, and other build tools from the builder stage. Since the dev and test stages inherit from builder, they won't have these tools available. This is efficient for image size but prevents installing additional packages with C extensions in development.If developers need to install additional packages in dev containers, consider creating a separate dev-builder stage that keeps build dependencies.
🔎 Alternative: Keep build-deps in dev stage
+# Builder without build deps removed +FROM builder AS builder-with-tools + # Optional dev environment -FROM builder AS dev +FROM builder-with-tools AS dev RUN --mount=type=cache,target=/root/.cache/uv \ uv sync --locked --group local CMD [ "./bin/docker-entrypoint-api-local.sh" ] +# Remove build deps for cleaner builder +FROM builder-with-tools AS builder +RUN apk del .build-depsNote: This is only needed if dev workflows require installing additional packages. The current design is fine if all dependencies are managed via uv/pyproject.toml.
59-84: Consider running as a non-root user for better security.The production stage doesn't include a
USERinstruction, so the container runs as root. While common, running as a non-root user is a security best practice that limits the impact of container breakouts or vulnerabilities.🔎 Proposed refactor to add non-root user
FROM python:3.12.10-alpine AS production # Install only runtime dependencies RUN apk add --no-cache \ bash \ libpq \ libxslt \ xmlsec +# Create non-root user +RUN addgroup -g 1001 -S plane && \ + adduser -u 1001 -S plane -G plane + # Copy the virtual environment from builder COPY --from=builder /app/.venv /app/.venv # Copy application code COPY --from=builder /app /app # Set working directory WORKDIR /app +# Change ownership to non-root user +RUN chown -R plane:plane /app + +# Switch to non-root user +USER plane + # Set PATH to include virtual environment ENV PATH="/app/.venv/bin:$PATH" # Expose Django default port EXPOSE 8000 CMD ["./bin/docker-entrypoint-api.sh"]
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/api/Dockerfile(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-25T10:17:39.709Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/bash.instructions.md:0-0
Timestamp: 2025-11-25T10:17:39.709Z
Learning: Applies to docker-compose.yml : Use docker-compose.yml for production and staging Docker configuration
Applied to files:
apps/api/Dockerfile
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (2)
apps/api/Dockerfile (2)
1-84: Well-structured multi-stage Dockerfile with past issues addressed.The Dockerfile is well-organized with clear separation between builder, dev, test, and production stages. Good practices observed:
- ✅ uv version pinned to 0.9.18 (addressing past concern)
- ✅ Runtime dependencies (libpq, libxslt, xmlsec) correctly installed in production (addressing past concern)
- ✅ Cache mounts used effectively for build performance
- ✅ Build dependencies properly isolated and cleaned up
- ✅ Python version pinned to 3.12.10-alpine
The migration from legacy Dockerfiles appears complete and production-ready, pending verification of the entrypoint script.
84-84: The entrypoint script correctly uses a production WSGI server.Verification confirms
./bin/docker-entrypoint-api.shlaunches gunicorn with uvicorn workers, not Django's development server. The implementation usesexec gunicorn -w "$GUNICORN_WORKERS" -k uvicorn.workers.UvicornWorker plane.asgi:applicationwith production settings (--max-requests 1200,--max-requests-jitter 1000,--access-logfile -) and proper signal handling viaexec.
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.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/api/Dockerfile (2)
59-61: Redundantapk del .build-deps— already removed in builder-with-tools.Line 42 already deletes
.build-depswithin thebuilder-with-toolsstage. This command is a no-op and can be removed, or the deletion at line 42 could be moved here for clearer separation of concerns.Option A: Remove redundant deletion
# Remove build deps for cleaner builder FROM builder-with-tools AS builder -RUN apk del .build-depsOption B: Move deletion from line 42 to here (cleaner separation)
At line 40-42:
RUN --mount=type=cache,target=/root/.cache/uv \ - uv sync --locked \ - && apk del .build-deps + uv sync --lockedThen keep line 61 as-is.
64-84: Production stage has redundant commands because it extendsbuilder.Since
productionusesFROM builder, it inherits all files and ENV frombuilder(which inherits frombuilder-with-tools). This makes several commands redundant:
Lines Command Why redundant 68-72 apk addruntime depsAlready installed in builder-with-tools(lines 23-27), never removed75-78 COPY --from=builderFiles already present via inheritance 84 ENV PATH=...Already set in builder-with-tools(line 37)If you intended a lean production image without build tools/uv, consider basing production on a fresh image:
🔎 Proposed lean production stage
# Stage 2: Production - Lean runtime image -FROM builder AS production +FROM python:3.12.10-alpine AS production # Install only runtime dependencies RUN apk add --no-cache \ bash=5.2.37-r0 \ libpq=17.7-r0 \ libxslt=1.1.43-r3 \ xmlsec=1.3.7-r0 # Copy the virtual environment from builder COPY --from=builder /app/.venv /app/.venv # Copy application code COPY --from=builder /app /app # Set working directory WORKDIR /app # Set PATH to include virtual environment ENV PATH="/app/.venv/bin:$PATH"This removes
uvand other build artifacts from the final image, reducing size.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/api/Dockerfile(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-25T10:17:39.709Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/bash.instructions.md:0-0
Timestamp: 2025-11-25T10:17:39.709Z
Learning: Applies to docker-compose.yml : Use docker-compose.yml for production and staging Docker configuration
Applied to files:
apps/api/Dockerfile
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (4)
apps/api/Dockerfile (4)
1-42: Builder stage looks well-structured.Good use of:
- Pinned uv version (0.9.18) for reproducibility
- Build cache mounts for faster rebuilds
- Separation of build deps (virtual package) and runtime deps
- Build deps cleanup after compilation
45-50: Dev stage inherits PATH correctly.The
devstage extendsbuilder-with-toolswhich setsPATH="/app/.venv/bin:$PATH"at line 37. Docker ENV directives are inherited, so the venv will be on PATH. Previous review concerns about missing PATH are resolved.
53-56: Test stage has no CMD defined.This is likely intentional to allow custom test commands via
docker run, but worth confirming this matches your test workflow.
86-89: Production entrypoint correctly uses the shell script.This addresses previous review concerns about using Django's development server. The
docker-entrypoint-api.shscript should handle gunicorn startup and initialization steps.
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/api/pyproject.toml (1)
23-23: Consider using a version range for consistency.The
fakerpackage is pinned to an exact version (==25.0.0) while all other dependencies use version ranges. For consistency with the rest of your dependency management strategy, consider using a range constraint like>=25.0.0,<26.0instead. The exact version will still be locked inuv.lockfor reproducibility.🔎 Suggested change for consistency
- "faker==25.0.0", + "faker>=25.0.0,<26.0",
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/api/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
apps/api/pyproject.toml(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (1)
apps/api/pyproject.toml (1)
1-5: LGTM! Project metadata properly updated.The project metadata changes align well with the migration to uv-based dependency management and the API consolidation objectives.
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/api/Dockerfile (1)
64-89: Simplify production stage by removing redundant operations.Since the production stage extends
builder(line 65), it already inherits the runtime libraries, virtual environment, application code, and PATH configuration. Lines 68-84 redundantly reinstall and recopy what's already present in the inherited filesystem.🔎 Simplified production stage
# Stage 2: Production - Lean runtime image FROM builder AS production - -# Install only runtime dependencies -RUN apk add --no-cache \ - bash=5.2.37-r0 \ - libpq=17.7-r0 \ - libxslt=1.1.43-r3 \ - xmlsec=1.3.7-r0 - -# Copy the virtual environment from builder -COPY --from=builder /app/.venv /app/.venv - -# Copy application code -COPY --from=builder /app /app - -# Set working directory -WORKDIR /app - -# Set PATH to include virtual environment -ENV PATH="/app/.venv/bin:$PATH" - -# Expose Django default port + EXPOSE 8000 - CMD ["./bin/docker-entrypoint-api.sh"]All the removed lines are redundant because
FROM builderalready provides these packages, files, and environment variables.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/api/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
apps/api/Dockerfile(1 hunks)apps/api/pyproject.toml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/pyproject.toml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-25T10:17:39.709Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/bash.instructions.md:0-0
Timestamp: 2025-11-25T10:17:39.709Z
Learning: Applies to docker-compose.yml : Use docker-compose.yml for production and staging Docker configuration
Applied to files:
apps/api/Dockerfile
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (4)
apps/api/Dockerfile (4)
2-42: LGTM: Builder stage correctly configured.The builder-with-tools stage properly:
- Pins uv version (addressing previous feedback)
- Sets PYTHONUNBUFFERED=1 for real-time container logs
- Installs both build dependencies and runtime libraries (bash, libpq, libxslt, xmlsec)
- Uses cache mounts for efficient dependency installation
- Configures virtual environment PATH
45-50: LGTM: Dev stage correctly inherits build environment.The dev stage appropriately extends builder-with-tools (which retains build dependencies needed for compiling any native extensions in the
localgroup), inherits the configured PATH, and runs the local development entrypoint script.
53-56: LGTM: Test stage correctly configured.The test stage appropriately extends builder-with-tools to install test group dependencies while retaining build tools for any compiled test packages.
59-61: LGTM: Builder cleanup stage correctly removes build dependencies.This stage correctly removes build-time dependencies after dev and test stages have branched off, creating a clean base for the production image.
| bash=5.2.37-r0 \ | ||
| libpq=17.7-r0 \ | ||
| libxslt=1.1.43-r3 \ | ||
| xmlsec=1.3.7-r0 |
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.
Hardcoded Alpine package versions will break builds
The Dockerfile pins exact Alpine package versions like gcc=14.2.0-r6, musl-dev=1.2.5-r10, python3-dev=3.12.12-r0, etc. These specific versions are tied to a particular Alpine release and will fail to install when the base python:3.12.10-alpine image uses a different Alpine version, or when building on different architectures (ARM vs x86). The build will fail with "no such package" errors. The old Dockerfile used flexible or no version constraints ("gcc", "bash~=5.2"), which worked across Alpine versions.
| # Expose Django default port | ||
| EXPOSE 8000 | ||
|
|
||
| CMD ["./bin/docker-entrypoint-api.sh"] No newline at end of file |
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.
Missing chmod makes entrypoint scripts non-executable
The old Dockerfile.api and Dockerfile.dev both explicitly ran chmod +x ./bin/* after copying files to ensure entrypoint scripts were executable. The new Dockerfile copies files with COPY . /code/ but never runs chmod. Both the dev and production stages use CMD to execute scripts from ./bin/ (docker-entrypoint-api.sh and docker-entrypoint-api-local.sh). If these scripts don't have execute permissions in the git repository, container startup will fail with "permission denied" errors.
Description
This PR adds
uvas package manager for theapiPython app.Dockerfile has been refactored to be multi-stage as well as producing "production", "dev" and "test" images
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
Built application, verified it's running
References
Note
Switch API to uv-managed pyproject and multi-stage Dockerfile, update CI to use new Dockerfile, and remove legacy requirements/Dockerfiles.
Dockerfile.api/Dockerfile.devwith a multi-stageapps/api/Dockerfileleveraginguvand dependency caching; addapps/api/.dockerignore.apps/api/pyproject.toml(runtime + local/test groups); removerequirements*files..github/workflows/build-branch.ymlto build API usingapps/api/Dockerfile.Written by Cursor Bugbot for commit 4e0316f. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.