-
Notifications
You must be signed in to change notification settings - Fork 287
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
build: switch backend base image - part 1 #1416
Conversation
WalkthroughThis pull request introduces modifications to the Docker configuration and dependency management in the backend project. The changes include updating the Dockerfile to use a slim Debian-based Python image, refining the Changes
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration 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.
Actionable comments posted: 1
♻️ Duplicate comments (1)
backend/Dockerfile (1)
12-41
: 🛠️ Refactor suggestionOptimize package installation and locale generation.
The package installation and locale generation can be optimized for better cache utilization and readability.
- Separate OS preparation from app build commands:
RUN apt-get update && apt-get install -y \ libyaml-cpp-dev \ libffi-dev \ libglib2.0-0 \ pango-1.0 \ libcairo2 \ locales \ tzdata \ gettext \ fontconfig \ fonts-freefont-ttf \ zlib1g-dev \ && apt-get clean \ - && rm -rf /var/lib/apt/lists/* \ - && locale-gen en_US.UTF-8 \ - && locale-gen ar_SA.UTF-8 \ - && locale-gen fr_FR.UTF-8 \ - && locale-gen pt_PT.UTF-8 \ - && locale-gen es_ES.UTF-8 \ - && locale-gen de_DE.UTF-8 \ - && locale-gen nl_NL.UTF-8 \ - && locale-gen it_IT.UTF-8 \ - && locale-gen pl_PL.UTF-8 \ - && locale-gen ro_RO.UTF-8 \ - && locale-gen hi_IN.UTF-8 \ - && locale-gen ur_PK.UTF-8 \ - && locale-gen cs_CZ.UTF-8 \ - && locale-gen sv_SE.UTF-8 \ - && locale-gen id_ID.UTF-8 \ - && pip install --no-cache-dir --upgrade pip poetry==2.0.1 + && rm -rf /var/lib/apt/lists/* +RUN locale-gen en_US.UTF-8 ar_SA.UTF-8 fr_FR.UTF-8 pt_PT.UTF-8 es_ES.UTF-8 \ + de_DE.UTF-8 nl_NL.UTF-8 it_IT.UTF-8 pl_PL.UTF-8 ro_RO.UTF-8 hi_IN.UTF-8 \ + ur_PK.UTF-8 cs_CZ.UTF-8 sv_SE.UTF-8 id_ID.UTF-8 +RUN pip install --no-cache-dir --upgrade pip poetry==2.0.1
- Consider adding comments to explain the purpose of each package group.
🧹 Nitpick comments (3)
backend/.dockerignore (1)
2-14
: LGTM! Consider adding more patterns for Python development.The added patterns effectively exclude cache directories and temporary files, which helps reduce the Docker build context size and improve build performance.
Consider adding these additional patterns commonly used in Python projects:
+.coverage +htmlcov/ +.mypy_cache/ +.env +*.egg-info/ +dist/ +build/backend/Dockerfile (2)
43-46
: Consider multi-stage build for production image.The current setup installs development dependencies in the final image. This will be addressed in part 2 with multi-stage builds.
For part 2, consider:
- Using multi-stage builds to separate build and runtime dependencies
- Creating a dedicated build stage for compiling dependencies
- Copying only necessary files to the final stage
48-49
: Add comment explaining the warning about local files.The comment about watching out for local files during dev could be more descriptive.
-#watch out for local files during dev and maintenance of .dockerignore +# Ensure .dockerignore is properly maintained to exclude unnecessary files +# that might be present in local development environment
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
backend/poetry.lock
is excluded by!**/*.lock
📒 Files selected for processing (3)
backend/.dockerignore
(1 hunks)backend/Dockerfile
(1 hunks)backend/pyproject.toml
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: enterprise-startup-functional-test (3.12)
- GitHub Check: enterprise-startup-docker-compose-test
- GitHub Check: startup-docker-compose-test
- GitHub Check: enterprise-functional-tests (3.12, chromium)
- GitHub Check: functional-tests (3.12, chromium)
- GitHub Check: test (3.12)
- GitHub Check: build (3.12)
- GitHub Check: startup-functional-test (3.12)
🔇 Additional comments (1)
backend/Dockerfile (1)
1-8
: LGTM! Environment variables are well organized.The switch to slim Debian base image and the consolidation of environment variables improve maintainability. The environment variables follow best practices for Poetry configuration.
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.
LGTM
Part one is to switch to slim (debian) instead of alpine for the dependencies issues
Part two will be to switch to multi stage build to optimize the size even more
Summary by CodeRabbit
Docker Configuration
.dockerignore
to exclude additional cache and temporary filesDependency Management
Chores