feat: auto-run database migrations on container startup#8
Open
damienriehl wants to merge 5 commits intomainfrom
Open
feat: auto-run database migrations on container startup#8damienriehl wants to merge 5 commits intomainfrom
damienriehl wants to merge 5 commits intomainfrom
Conversation
- Dockerfile.prod: add git/libgit2-dev deps, /data/repos dir, pre-download sentence-transformers model - config.py: auto-convert postgresql:// to postgresql+asyncpg:// for Railway - railway.json: use Dockerfile.prod and run Alembic migrations on deploy Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Redis pool.ping() has no default socket timeout, causing the API to hang indefinitely if Redis is unreachable at startup. Similarly, asyncpg has no connect timeout by default. Add explicit timeouts so startup gracefully degrades instead of blocking forever. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The .env file was being copied into the Docker image by railway up, causing Pydantic Settings to read localhost URLs instead of Railway's injected environment variables. This made the API hang on startup trying to connect to localhost:5432/6379 inside the container. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add entrypoint script that runs `alembic upgrade head` before starting the application. Prevents schema drift when the API image or mounted code has new migrations but the database hasn't been migrated. - New scripts/entrypoint.sh: runs migrations then exec's CMD Uses /tmp marker to skip on uvicorn --reload re-invocations - Dockerfile: ENTRYPOINT delegates to entrypoint.sh - compose.yaml: mount entrypoint.sh + set entrypoint/command for dev Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughChanges implement automated database migrations on container startup via a new entrypoint script and update Docker configurations to support this flow. Additional timeouts are configured for PostgreSQL and Redis connections, and Railway deployment configuration is added with production Dockerfile enhancements including git dependencies and pre-cached ML models. Changes
Sequence DiagramsequenceDiagram
participant Container as Container Startup
participant Entrypoint as entrypoint.sh
participant Alembic as Alembic Migrations
participant Database as PostgreSQL DB
participant Uvicorn as Uvicorn App Server
Container->>Entrypoint: Execute ENTRYPOINT
Entrypoint->>Entrypoint: Check /tmp/.migrations_done marker
alt Marker does not exist
Entrypoint->>Alembic: Run 'alembic upgrade head'
Alembic->>Database: Apply pending migrations
Database-->>Alembic: Migration complete
Alembic-->>Entrypoint: Exit success
Entrypoint->>Entrypoint: Create /tmp/.migrations_done marker
else Marker exists
Entrypoint->>Entrypoint: Skip migrations
end
Entrypoint->>Uvicorn: exec CMD (uvicorn on 0.0.0.0:8000)
Uvicorn->>Database: Connect with 30s timeout
Database-->>Uvicorn: Connected
Uvicorn-->>Container: App running
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
Dockerfile (1)
50-52: Consider using absolute path in ENTRYPOINT.The entrypoint relies on
/usr/local/binbeing in PATH. While this works, using the absolute path is more explicit and avoids potential PATH-related issues:Suggested change
-ENTRYPOINT ["entrypoint.sh"] +ENTRYPOINT ["/usr/local/bin/entrypoint.sh"]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` around lines 50 - 52, The ENTRYPOINT currently uses a relative/ PATH-resolved script name "entrypoint.sh", which can fail if PATH doesn't include /usr/local/bin; update the Dockerfile ENTRYPOINT to reference the script with its absolute path (e.g., /usr/local/bin/entrypoint.sh) and ensure the referenced file (entrypoint.sh) is present in the image at that absolute location and marked executable; target the ENTRYPOINT symbol and the entrypoint.sh file when making the change.Dockerfile.prod (1)
50-51: Production Dockerfile lacks entrypoint for auto-migrations.Unlike the development
Dockerfilewhich usesENTRYPOINT ["entrypoint.sh"]to run migrations on startup,Dockerfile.prodrelies solely on Railway'sreleaseCommand. If this image is deployed outside Railway (e.g., Kubernetes, ECS), migrations won't run automatically.Consider adding the entrypoint script for consistency across deployment targets:
Add entrypoint to Dockerfile.prod
# Copy alembic configuration for migrations COPY --chown=ontokit:ontokit alembic.ini ./ COPY --chown=ontokit:ontokit alembic/ ./alembic/ +# Copy entrypoint script (runs migrations before starting the app) +COPY --chown=ontokit:ontokit scripts/entrypoint.sh /usr/local/bin/entrypoint.sh +RUN chmod +x /usr/local/bin/entrypoint.sh + # Switch to non-root user USER ontokitAnd update the CMD to use ENTRYPOINT:
+ENTRYPOINT ["/usr/local/bin/entrypoint.sh"] CMD ["uvicorn", "ontokit.main:app", "--host", "0.0.0.0", "--port", "8000", "--workers", "4"]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile.prod` around lines 50 - 51, Dockerfile.prod currently only sets CMD to run uvicorn, so automatic migrations (handled by entrypoint.sh in dev) won't run when deployed outside Railway; add an ENTRYPOINT that points to the existing entrypoint.sh before the CMD line and ensure the entrypoint.sh is executable and performs the migrations then execs the CMD command (so uvicorn receives signals). Update Dockerfile.prod to include ENTRYPOINT ["./entrypoint.sh"] (or the correct path used in the repo) and keep the CMD ["uvicorn", "ontokit.main:app", "--host", "0.0.0.0", "--port", "8000", "--workers", "4"] so entrypoint.sh runs migrations and then execs uvicorn.railway.json (1)
1-9: Redundant migration execution between release command and entrypoint.The
releaseCommandrunsalembic upgrade headduring Railway deployment, whilescripts/entrypoint.shalso runs the same command on container start. This creates redundant migration attempts:
- Railway runs
releaseCommandbefore deployment- New containers start and run
entrypoint.shwhich also callsalembic upgrade headAlembic uses PostgreSQL advisory locks to prevent concurrent execution, so this is safe but wasteful. Consider either:
- Removing the
releaseCommandand relying solely on the entrypoint (consistent across all deployment targets)- Or setting an environment variable in Railway to skip the entrypoint migration check
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@railway.json` around lines 1 - 9, The deploy config runs "alembic upgrade head" via the releaseCommand while scripts/entrypoint.sh runs the same migration on container start, causing redundant attempts; fix by choosing one strategy: either remove the "releaseCommand": "alembic upgrade head" from railway.json so migrations only run in scripts/entrypoint.sh (locate the releaseCommand key in railway.json and delete it), or set a Railway environment variable (e.g., SKIP_ENTRYPOINT_MIGRATIONS=true) and update scripts/entrypoint.sh to skip running alembic when that env var is set (modify the migration logic in scripts/entrypoint.sh to check SKIP_ENTRYPOINT_MIGRATIONS before calling alembic upgrade head).scripts/entrypoint.sh (2)
7-11: Gate migration execution to avoid replica startup contention.This guard is container-local only. With multiple app replicas, each instance can still run Alembic on boot. Prefer env-gating here so only one designated instance/job performs migrations.
Proposed change
MARKER="/tmp/.migrations_done" -if [ ! -f "$MARKER" ]; then +RUN_MIGRATIONS="${RUN_MIGRATIONS:-1}" +if [ "$RUN_MIGRATIONS" = "1" ] && [ ! -f "$MARKER" ]; then echo "Running database migrations..." python -m alembic upgrade head touch "$MARKER" +elif [ "$RUN_MIGRATIONS" != "1" ]; then + echo "Skipping migrations (RUN_MIGRATIONS=$RUN_MIGRATIONS)" fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/entrypoint.sh` around lines 7 - 11, The script currently runs Alembic migrations based only on a container-local MARKER which allows multiple replicas to run migrations concurrently; update the entrypoint.sh to gate running "python -m alembic upgrade head" behind an environment flag (e.g., RUN_MIGRATIONS or MIGRATION_LEADER) so only the designated instance performs migrations: check that the env var is set to "true" (or non-empty) AND MARKER does not exist before executing the alembic command, otherwise skip migrations; keep the existing MARKER touch logic (touch "$MARKER") after a successful run so subsequent restarts of the same container still skip re-running migrations, and ensure the conditional uses the same variable name you choose so callers can control which replica/job runs migrations.
2-2: Harden shell error handling flags.Use strict mode (
set -euo pipefail) to fail on unset variables and pipe failures too, not just non-zero exits.Proposed change
-set -e +set -euo pipefail🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/entrypoint.sh` at line 2, Replace the existing shell invocation "set -e" with strict mode flags "set -euo pipefail" in the entrypoint script so the shell fails on unset variables and pipe failures as well as non-zero exits; locate the line containing "set -e" and change it to "set -euo pipefail".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Dockerfile.prod`:
- Around line 30-31: The pre-download RUN that invokes
SentenceTransformer('all-MiniLM-L6-v2') runs as root so the model cache lives
under root and the app (which runs as USER ontokit) cannot access it; either
move that RUN to after the Dockerfile USER ontokit switch so the model is cached
into ontokit’s home, or keep it as root but set an explicit shared cache
directory (via env vars like TRANSFORMERS_CACHE or HF_HOME) and ensure
permissions are adjusted and the ontokit user owns that cache directory so
SentenceTransformer can find the model at runtime.
In `@ontokit/core/config.py`:
- Around line 35-41: The method convert_postgres_scheme is missing type
annotations; add typing to satisfy MyPy by importing Any from typing and
annotating the signature as `@classmethod` def convert_postgres_scheme(cls: type,
v: Any) -> Any so the validator's pre-processing input and output are typed;
keep the existing logic and return types unchanged.
---
Nitpick comments:
In `@Dockerfile`:
- Around line 50-52: The ENTRYPOINT currently uses a relative/ PATH-resolved
script name "entrypoint.sh", which can fail if PATH doesn't include
/usr/local/bin; update the Dockerfile ENTRYPOINT to reference the script with
its absolute path (e.g., /usr/local/bin/entrypoint.sh) and ensure the referenced
file (entrypoint.sh) is present in the image at that absolute location and
marked executable; target the ENTRYPOINT symbol and the entrypoint.sh file when
making the change.
In `@Dockerfile.prod`:
- Around line 50-51: Dockerfile.prod currently only sets CMD to run uvicorn, so
automatic migrations (handled by entrypoint.sh in dev) won't run when deployed
outside Railway; add an ENTRYPOINT that points to the existing entrypoint.sh
before the CMD line and ensure the entrypoint.sh is executable and performs the
migrations then execs the CMD command (so uvicorn receives signals). Update
Dockerfile.prod to include ENTRYPOINT ["./entrypoint.sh"] (or the correct path
used in the repo) and keep the CMD ["uvicorn", "ontokit.main:app", "--host",
"0.0.0.0", "--port", "8000", "--workers", "4"] so entrypoint.sh runs migrations
and then execs uvicorn.
In `@railway.json`:
- Around line 1-9: The deploy config runs "alembic upgrade head" via the
releaseCommand while scripts/entrypoint.sh runs the same migration on container
start, causing redundant attempts; fix by choosing one strategy: either remove
the "releaseCommand": "alembic upgrade head" from railway.json so migrations
only run in scripts/entrypoint.sh (locate the releaseCommand key in railway.json
and delete it), or set a Railway environment variable (e.g.,
SKIP_ENTRYPOINT_MIGRATIONS=true) and update scripts/entrypoint.sh to skip
running alembic when that env var is set (modify the migration logic in
scripts/entrypoint.sh to check SKIP_ENTRYPOINT_MIGRATIONS before calling alembic
upgrade head).
In `@scripts/entrypoint.sh`:
- Around line 7-11: The script currently runs Alembic migrations based only on a
container-local MARKER which allows multiple replicas to run migrations
concurrently; update the entrypoint.sh to gate running "python -m alembic
upgrade head" behind an environment flag (e.g., RUN_MIGRATIONS or
MIGRATION_LEADER) so only the designated instance performs migrations: check
that the env var is set to "true" (or non-empty) AND MARKER does not exist
before executing the alembic command, otherwise skip migrations; keep the
existing MARKER touch logic (touch "$MARKER") after a successful run so
subsequent restarts of the same container still skip re-running migrations, and
ensure the conditional uses the same variable name you choose so callers can
control which replica/job runs migrations.
- Line 2: Replace the existing shell invocation "set -e" with strict mode flags
"set -euo pipefail" in the entrypoint script so the shell fails on unset
variables and pipe failures as well as non-zero exits; locate the line
containing "set -e" and change it to "set -euo pipefail".
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a8323202-fd9c-4de1-85c8-ecbbf810c7ae
📒 Files selected for processing (9)
.dockerignoreDockerfileDockerfile.prodcompose.yamlontokit/core/config.pyontokit/core/database.pyontokit/main.pyrailway.jsonscripts/entrypoint.sh
Comment on lines
+30
to
+31
| # Pre-download sentence-transformers model to avoid first-request latency | ||
| RUN python -c "from sentence_transformers import SentenceTransformer; SentenceTransformer('all-MiniLM-L6-v2')" |
There was a problem hiding this comment.
Model downloaded as root, but app runs as non-root user.
The model is pre-downloaded before USER ontokit (line 41), so it's cached under root's home directory. When the app runs as ontokit, sentence-transformers may not find the cached model and re-download it on first use.
Move the model download after switching to the ontokit user, or set the cache directory explicitly:
Option 1: Move after USER switch
# Switch to non-root user
USER ontokit
+# Pre-download sentence-transformers model to avoid first-request latency
+RUN python -c "from sentence_transformers import SentenceTransformer; SentenceTransformer('all-MiniLM-L6-v2')"
+
# Expose port
EXPOSE 8000Option 2: Use explicit cache directory
# Pre-download sentence-transformers model to avoid first-request latency
-RUN python -c "from sentence_transformers import SentenceTransformer; SentenceTransformer('all-MiniLM-L6-v2')"
+ENV SENTENCE_TRANSFORMERS_HOME=/home/ontokit/.cache/sentence-transformers
+RUN mkdir -p $SENTENCE_TRANSFORMERS_HOME && \
+ python -c "from sentence_transformers import SentenceTransformer; SentenceTransformer('all-MiniLM-L6-v2')" && \
+ chown -R ontokit:ontokit /home/ontokit/.cache📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # Pre-download sentence-transformers model to avoid first-request latency | |
| RUN python -c "from sentence_transformers import SentenceTransformer; SentenceTransformer('all-MiniLM-L6-v2')" | |
| # Pre-download sentence-transformers model to avoid first-request latency | |
| ENV SENTENCE_TRANSFORMERS_HOME=/home/ontokit/.cache/sentence-transformers | |
| RUN mkdir -p $SENTENCE_TRANSFORMERS_HOME && \ | |
| python -c "from sentence_transformers import SentenceTransformer; SentenceTransformer('all-MiniLM-L6-v2')" && \ | |
| chown -R ontokit:ontokit /home/ontokit/.cache |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Dockerfile.prod` around lines 30 - 31, The pre-download RUN that invokes
SentenceTransformer('all-MiniLM-L6-v2') runs as root so the model cache lives
under root and the app (which runs as USER ontokit) cannot access it; either
move that RUN to after the Dockerfile USER ontokit switch so the model is cached
into ontokit’s home, or keep it as root but set an explicit shared cache
directory (via env vars like TRANSFORMERS_CACHE or HF_HOME) and ensure
permissions are adjusted and the ontokit user owns that cache directory so
SentenceTransformer can find the model at runtime.
Comment on lines
+35
to
+41
| @field_validator("database_url", mode="before") | ||
| @classmethod | ||
| def convert_postgres_scheme(cls, v): | ||
| """Railway provides postgresql:// but SQLAlchemy async needs postgresql+asyncpg://.""" | ||
| if isinstance(v, str) and v.startswith("postgresql://"): | ||
| return v.replace("postgresql://", "postgresql+asyncpg://", 1) | ||
| return v |
There was a problem hiding this comment.
Add type annotations to fix MyPy error.
The pipeline fails with mypy: Function is missing a type annotation [no-untyped-def]. Per coding guidelines, MyPy strict mode is enforced. Add type hints:
Proposed fix
`@field_validator`("database_url", mode="before")
`@classmethod`
- def convert_postgres_scheme(cls, v):
+ def convert_postgres_scheme(cls, v: object) -> object:
"""Railway provides postgresql:// but SQLAlchemy async needs postgresql+asyncpg://."""
if isinstance(v, str) and v.startswith("postgresql://"):
return v.replace("postgresql://", "postgresql+asyncpg://", 1)
return vAs per coding guidelines: "Enable and enforce MyPy strict mode type checking with Python 3.11 target".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @field_validator("database_url", mode="before") | |
| @classmethod | |
| def convert_postgres_scheme(cls, v): | |
| """Railway provides postgresql:// but SQLAlchemy async needs postgresql+asyncpg://.""" | |
| if isinstance(v, str) and v.startswith("postgresql://"): | |
| return v.replace("postgresql://", "postgresql+asyncpg://", 1) | |
| return v | |
| `@field_validator`("database_url", mode="before") | |
| `@classmethod` | |
| def convert_postgres_scheme(cls, v: object) -> object: | |
| """Railway provides postgresql:// but SQLAlchemy async needs postgresql+asyncpg://.""" | |
| if isinstance(v, str) and v.startswith("postgresql://"): | |
| return v.replace("postgresql://", "postgresql+asyncpg://", 1) | |
| return v |
🧰 Tools
🪛 GitHub Actions: Distribution
[error] 37-37: mypy: Function is missing a type annotation [no-untyped-def]
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ontokit/core/config.py` around lines 35 - 41, The method
convert_postgres_scheme is missing type annotations; add typing to satisfy MyPy
by importing Any from typing and annotating the signature as `@classmethod` def
convert_postgres_scheme(cls: type, v: Any) -> Any so the validator's
pre-processing input and output are typed; keep the existing logic and return
types unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
scripts/entrypoint.sh): runsalembic upgrade headbefore starting the application, with a/tmpmarker file to skip on uvicorn--reloadre-invocationsENTRYPOINTdelegates to entrypoint.sh,CMDremains uvicornentrypoint/commandso it works without rebuilding the imageContext
An 18MB OWL import was failing with "Could not reach the server" — root cause was a missing
github_hook_idcolumn (4 migrations behind). The unhandled 500 lacked CORS headers, causing the browser to treat it as a network error. This entrypoint prevents schema drift by auto-migrating on every container start.Test plan
docker compose up -d api— logs should show "Running database migrations..." once before uvicorn startsdocker compose upfrom rebuilt image — migrations run from ENTRYPOINT🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements