-
Notifications
You must be signed in to change notification settings - Fork 18
feat(container): add Docker Compose support with persistent SQLite volume #361
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
Conversation
|
Warning Rate limit exceeded@nanotaboada has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 52 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (9)
WalkthroughThe changes migrate the application from using a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Docker Compose
participant Container
participant Entrypoint Script
participant SQLite DB Volume
User->>Docker Compose: docker compose up
Docker Compose->>Container: Start container
Container->>Entrypoint Script: Run entrypoint.sh
Entrypoint Script->>SQLite DB Volume: Check for DB file
alt DB file missing in volume
Entrypoint Script->>Container: Copy DB from image to volume
else DB file exists
Entrypoint Script->>Entrypoint Script: Skip copy
end
Entrypoint Script->>Container: Start Uvicorn server (CMD)
Possibly related PRs
✨ Finishing Touches
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 (
|
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #361 +/- ##
=======================================
Coverage 89.18% 89.18%
=======================================
Files 2 2
Lines 111 111
=======================================
Hits 99 99
Misses 12 12
🚀 New features to boost your workflow:
|
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 (10)
Dockerfile (2)
49-51: Clarify seed DB copy destination
Copying your seed database fromsqlite3-dbinto/app/docker-composeworks, but naming a folderdocker-composeinside your image may be confusing. Consider renaming the target dir to something likeseed-dbfor clarity, e.g.:- COPY --chown=root:root --chmod=755 sqlite3-db ./docker-compose + COPY --chown=root:root --chmod=755 sqlite3-db ./seed-dbThen update
ENTRYPOINT,entrypoint.sh, and your code to reference/app/seed-db/players-sqlite3.db.
66-66: Use absolute path in ENTRYPOINT
Relying on./entrypoint.shworks because ofWORKDIR, but using the absolute path makes it more robust:- ENTRYPOINT ["./entrypoint.sh"] + ENTRYPOINT ["/app/entrypoint.sh"]docker-compose.yml (3)
1-4: Specify Compose file version for compatibility
Although modern Docker Compose can infer the format, adding at the top:version: "3.9"ensures consistent behavior across different Docker/Compose versions.
5-12: Use external.envfor configuration
HardcodingDATABASE_FILE_PATHin the Compose file reduces flexibility. You can move it to a.envfile:env_file: - .env environment: - DATABASE_FILE_PATH=${DATABASE_FILE_PATH}This makes overrides and local testing easier.
1-19: Add a healthcheck for liveness/readiness
To improve operational resilience, define ahealthcheckthat probes your FastAPI app (e.g.,curl --fail http://localhost:9000/health). Docker can then automatically restart the container on failure.scripts/entrypoint.sh (2)
1-3: Enhance shell safety
You haveset -e, but consider using:#!/usr/bin/env bash set -euo pipefailThis will also catch unset variables and pipeline failures.
4-6: Respect theDATABASE_FILE_PATHenv var
Right nowVOLUME_DATABASE_FILE_PATHis hardcoded. To allow overrides, use something like:IMAGE_DATABASE_FILE_PATH="/app/seed-db/players-sqlite3.db" VOLUME_DATABASE_FILE_PATH="${DATABASE_FILE_PATH:-/sqlite3-db/players-sqlite3.db}"This makes your script adapt if someone changes the env var.
README.md (3)
62-64: Minor grammar: add missing article
Change:> On first run, the container...to:
> On the first run, the container...🧰 Tools
🪛 LanguageTool
[uncategorized] ~62-~62: You might be missing the article “the” here.
Context: ...ppbash docker compose up> On first run, the container copies a pre-seeded ...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
44-54: Document environment variable overrides
Mention how consumers can overrideDATABASE_FILE_PATHvia a.envfile ordocker compose up --env-file:You can set `DATABASE_FILE_PATH` in a `.env` file to point to a custom DB location.
56-60: Include detached mode example
Add a note showing how to run in detached mode:docker compose up -dso the terminal remains free.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
sqlite3-db/players-sqlite3.dbis excluded by!**/*.db
📒 Files selected for processing (9)
.codacy.yml(1 hunks)Dockerfile(4 hunks)README.md(1 hunks)codecov.yml(1 hunks)database/player_database.py(1 hunks)docker-compose.yml(1 hunks)routes/player_route.py(1 hunks)schemas/player_schema.py(1 hunks)scripts/entrypoint.sh(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
routes/player_route.py (1)
database/player_database.py (1)
generate_async_session(33-41)
🪛 GitHub Check: Codeac Code Quality
database/player_database.py
[failure] 8-8: import-error
Unable to import 'sqlalchemy.ext.asyncio'
[failure] 9-9: import-error
Unable to import 'sqlalchemy.orm'
🪛 LanguageTool
README.md
[uncategorized] ~62-~62: You might be missing the article “the” here.
Context: ...pp bash docker compose up > On first run, the container copies a pre-seeded ...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
🔇 Additional comments (13)
codecov.yml (1)
43-43: Properly updated ignore path for project restructuring.The change from
^data/.*to^database/.*in the ignore paths aligns with the project's directory restructuring where database-related files are moved from thedatatodatabasedirectory..codacy.yml (1)
5-5: Consistently updated exclude path.The exclusion path update from
data/**/*todatabase/**/*correctly maintains consistency with the project's directory restructuring and matches the changes made in codecov.yml.routes/player_route.py (1)
10-10: Import path properly updated for new directory structure.The import path has been correctly updated to reflect the migration from
datatodatabasedirectory, maintaining the same functionality while supporting the restructured project organization.schemas/player_schema.py (1)
6-6: Import path correctly updated for directory restructuring.The import statement for
Basehas been appropriately updated to reference the newdatabasedirectory, maintaining consistency with other changes in this PR.database/player_database.py (2)
6-6: Added os module import for environment variable support.The
osmodule import enables environment variable access, which is essential for containerized applications to configure database paths externally.
11-12:Details
✅ Verification successful
Environment variable support added for database path configuration.
This change effectively implements environment variable configuration for the database file path, with a sensible default that points to the expected volume mount location. This is a crucial component for Docker Compose deployments with persistent volumes.
For the SQLite path, using environment variables allows the container to connect to the database in the mounted volume while maintaining the same code in different environments.
🏁 Script executed:
#!/bin/bash # Verify if there are any references to the old database path that need updating echo "Checking for any remaining references to old database path..." rg -i "data/players-sqlite3.db" --type-not=ymlLength of output: 207
🏁 Script executed:
#!/bin/bash echo "Searching for any remaining references to the old database path across the repository..." # Remove the unrecognized --type-not filter to scan all files rg -i "data/players-sqlite3.db"Length of output: 218
No Remaining References to Old Database Path
Verified that a repository-wide search for “data/players-sqlite3.db” returned no matches. The new environment-variable-driven default (
./sqlite3-db/players-sqlite3.db) is fully in place, and there are no lingering references to the old path.Dockerfile (4)
17-18: Skip: Stage comments added
The new Stage 2 comments clearly delineate the runtime phase. No action needed here.
32-43: Skip: Code COPY directives look correct
The application code and prebuilt wheels are copied in read-only mode with appropriate ownership and permissions. Everything aligns with the multi-stage build approach.
45-45: Skip: README metadata copy
Copying theREADME.mdinto the image for reference is fine.
53-57: Non-root user and volume mount setup is solid
Creating afastapisystem user/group and pre-chowning/sqlite3-dbpromotes security. Well done.docker-compose.yml (1)
17-19: Volume declaration is correct
Defining thesqlite3-dbnamed volume at the bottom ensures persistence. No changes needed.scripts/entrypoint.sh (1)
25-26: Exec replacement is correct
Usingexec "$@"cleanly replaces the shell with your Uvicorn command, ensuring proper signal handling.README.md (1)
44-49: Documentation updated for Docker Compose
The new “Container” section accurately replaces the old Docker instructions with Docker Compose steps. Nice clarity.
4345e1e to
2628fed
Compare
…i-restful into feature/docker-compose
|



This change is
Summary by CodeRabbit
New Features
Improvements
Documentation
Chores