-
Notifications
You must be signed in to change notification settings - Fork 121
Update docker images and build images #655
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: main
Are you sure you want to change the base?
Conversation
WalkthroughThe updates modify Dockerfiles for both the backend and database components. The backend's base image changes to Changes
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (1)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
🔭 Outside diff range comments (1)
Dockerfile (1)
59-69
: 🛠️ Refactor suggestionReduce image size by removing build dependencies.
You installgcc
,build-essential
, andlibpq-dev
in the final image, but they are only needed for building your Python dependencies and Node modules. Consider removing these packages after building, or better yet, move all build-related steps into a dedicated build stage (multi-stage build) to keep the final image lean.Proposed diff snippet:
RUN apt-get update && \ - apt-get install -y --no-install-recommends \ + apt-get install -y --no-install-recommends build-deps \ cron \ postgresql \ redis-server \ @@ && yarn install --frozen-lockfile --network-timeout 1000000 && yarn build \ - && rm -rf ./artifacts \ + && rm -rf ./artifacts \ + # remove build dependencies + && apt-get purge -y build-deps \ + && apt-get autoremove -y \ && cd /app/ \ && tar cfz web-server.tar.gz ./web-server \
🧹 Nitpick comments (5)
Dockerfile (2)
76-79
: Confirm PostgreSQL data directory paths.
You're appending Postgres configs under/etc/postgresql/15/main/
. In this image you install thepostgresql
meta-package (Bookworm defaults to v15), so this path exists. However, if the default ever moves, these hardcoded paths will break. Consider pinningpostgresql-15
explicitly or templating the version.
80-89
: Unifydbmate
architecture detection.
In the backend Dockerfile, you branch onuname -m
for each arch. You can simplify by usingdpkg --print-architecture
(as in the database image) to fetchamd64
,arm64
, etc., and use a single URL template. This reduces duplication and supports all Debian-supported arches.Example:
- if [ "$(uname -m)" = "x86_64" ]; then \ - ARCH=amd64; \ - elif [ "$(uname -m)" = "aarch64" ] || [ "$(uname -m)" = "arm64" ]; then \ - ARCH=arm64; \ - else \ - echo "Unsupported architecture: $(uname -m)"; exit 1; \ - fi && \ - curl -fsSL -o /usr/local/bin/dbmate \ - https://github.com/amacneil/dbmate/releases/download/v1.16.0/dbmate-linux-${ARCH}; \ + ARCH=$(dpkg --print-architecture) && \ + curl -fsSL -o /usr/local/bin/dbmate \ + https://github.com/amacneil/dbmate/releases/download/v1.16.0/dbmate-linux-${ARCH} && \database-docker/Dockerfile (3)
4-4
: Lock PostgreSQL repository to Bookworm PGDG.
You updated the PGDG source tobookworm-pgdg
, which is correct. Ensure that the GPG key you add corresponds to this repository; you've added it viaapt-key
, which is deprecated in newer Debian versions.Consider using:
RUN curl -fsSL https://www.postgresql.org/media/keys/ACCC4CF8.asc \ | gpg --dearmor > /usr/share/keyrings/postgresql-archive-keyring.gpg && \ echo "deb [signed-by=/usr/share/keyrings/postgresql-archive-keyring.gpg] \ http://apt.postgresql.org/pub/repos/apt/ bookworm-pgdg main" \ > /etc/apt/sources.list.d/pgdg.list
8-10
: Remove duplicate key import commands.
You're adding the same PostgreSQL GPG key twice (viacurl
andwget
). One import is sufficient and will speed up the build.- RUN curl -sSL https://www.postgresql.org/media/keys/ACCC4CF8.asc | apt-key add - - RUN wget --quiet -O - https://www.postgresql.org/media/keys/ACCC4CF8.asc | apt-key add - + RUN curl -sSL https://www.postgresql.org/media/keys/ACCC4CF8.asc | apt-key add -
13-15
: Usedpkg --print-architecture
fordbmate
.
Great enhancement to dynamically fetch the correctdbmate
binary. As a refinement, add a check to fail early if the URL for the architecture is unavailable.Example:
RUN ARCH=$(dpkg --print-architecture) && \ URL="https://github.com/amacneil/dbmate/releases/download/v1.16.0/dbmate-linux-${ARCH}" && \ curl --fail --location -o /usr/local/bin/dbmate "$URL" && \ chmod +x /usr/local/bin/dbmate
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (2)
Dockerfile
(2 hunks)database-docker/Dockerfile
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: All file linting
🔇 Additional comments (5)
Dockerfile (2)
14-14
: Update base image to Bookworm for backend-build.
Thepython:3.9-slim-bookworm
image replaces the previous Debian slim release, which keeps the OS up-to-date. Ensure compatibility of installed packages (e.g.,libpq-dev
) with Bookworm.
59-117
:✅ Verification successful
Validate
start.sh
extraction logic.
Your final RUN step tars/opt/venv
and/app/web-server
and then removes their contents, relying onsetup_utils/start.sh
(CMD) to extract them at container start. Please ensure thatstart.sh
correctly unpacks these tarballs before launching services; otherwise, the application environment will be missing critical files.To verify, search for extraction commands in
start.sh
:
🏁 Script executed:
#!/bin/bash rg -n "tar -xzf" -- app/setup_utils/start.shLength of output: 111
🏁 Script executed:
#!/bin/bash set -e # 1. Locate any start.sh scripts in the repo find . -type f -name "start.sh" # 2. Confirm which startup script the Dockerfile actually invokes rg -n '^CMD' -- DockerfileLength of output: 149
🏁 Script executed:
#!/bin/bash set -e # Display the contents of the startup script to verify extraction logic echo "----- setup_utils/start.sh -----" sed -n '1,200p' setup_utils/start.sh # Highlight any tar commands within the script echo "----- Tar commands in start.sh -----" grep -n "tar" setup_utils/start.sh || echo "No tar commands found"Length of output: 1904
Extraction logic in start.sh is correctly implemented.
We’ve confirmed thatsetup_utils/start.sh
checks for/opt/venv.tar.gz
and/app/web-server.tar.gz
, extracts each withtar xzf … --strip-components=2
into the proper directory, and then removes the tarball before starting Supervisor. No further changes are needed.database-docker/Dockerfile (3)
1-4
: Upgrade base image to Debian Bookworm.
SwitchingFROM debian:bookworm
ensures you’re on the non-deprecated Bookworm release. Confirm that any Debian-specific package names and repository URLs in this file match Bookworm conventions.
11-12
: Explicitly installpostgresql-15
.
Installingpostgresql-15
directly locks the version and aligns with your config paths. This avoids ambiguity if the defaultpostgresql
meta-package ever moves to another version.
20-20
: Update PATH to include PostgreSQL 15 binaries.
UpdatingENV PATH
to/usr/lib/postgresql/15/bin:$PATH
exposes the correct Postgres tools. Ensure no older Postgres binaries remain in the image.
Linked Issue(s)
Related to #619 (comment)
The above comment mentioned about buster being depricated, so this PR attempts to update it
Further comments
Summary by CodeRabbit