fix(compose): stop Marker PDF conversion failing with PermissionError#655
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughDocker Compose defines a shared ChangesCompose environment configuration
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@infra/compose/docker-compose.yaml`:
- Around line 7-18: FONT_PATH is not consumed by Marker, so the font remains in
the package directory. Update the Marker configuration or conversion path used
by the PDF conversion service to pass the supported font-location override, and
adjust the shared `openrag_env` setting to that supported key.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 066f3af0-662a-4de7-a7da-b523ec651b66
📒 Files selected for processing (1)
infra/compose/docker-compose.yaml
There was a problem hiding this comment.
Looks good to me overall. The compose change fixes the Marker font path in the right place, and the rendered config keeps both the CPU/GPU services covered.
One small non-blocking suggestion: because FONT_PATH is now set directly in environment, it will override any value coming from .env. Could we make it overridable like this?
FONT_PATH: ${FONT_PATH:-/app/data/fonts/GoNotoCurrent-Regular.ttf}
That keeps the default fix exactly the same, but still gives operators an escape hatch if they need to use a custom font path later.
There was a problem hiding this comment.
Approved. The latest commit fixes the FONT_PATH override concern, and the rendered compose config now keeps the default path while still allowing a custom value from the environment. I also checked the CPU/GPU config render and the infra tests; everything looks good to me.
16895bd to
b18d896
Compare
Marker downloads its rendering font lazily on the first PDF conversion,
and its default target is inside the installed package:
site-packages/static/fonts/GoNotoCurrent-Regular.ttf
entrypoint.sh drops the app to a non-root user (APP_UID, GID 0) while
the venv lives in the root-owned `openrag_venv` named volume, whose
site-packages is `root:root drwxr-xr-x`. The font download therefore
cannot create `static/` and every PDF ingestion dies with:
PermissionError: [Errno 13] Permission denied:
'/app/.venv/lib/python3.12/site-packages/static'
Point FONT_PATH at /app/data/fonts/, a bind-mounted directory the app
user already owns (entrypoint.sh grants GID-0 write on /app/data). The
~15MB download then happens once and persists across restarts.
FONT_PATH is the only knob needed: marker's Settings is a pydantic
BaseSettings with no env prefix, FONT_DIR is dead except as the default
for FONT_PATH, and the two other readers (providers/__init__.py,
processors/debug.py) both go through FONT_PATH.
The shared env is hoisted into its own `x-openrag-env` anchor because a
service's own `environment:` key *replaces* the one merged in via `<<:`
(YAML merge is shallow). Adding it to `x-openrag` alone was silently
dropped by the GPU `openrag` service, which declares its own
`environment:`. Both variants now merge the anchor explicitly, and the
GPU service's list form is converted to mapping form to allow it.
Verified on a live stack: a PDF ingested end-to-end reaches
task_state=COMPLETED, marker logs "Processed ... in 33.75s", and the
PermissionError count since restart is zero.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The x-openrag-env anchor hardcoded FONT_PATH, so it always won over
any value set in .env. Fall back to the same default via ${FONT_PATH:-...}
so operators still have an escape hatch for a custom font location.
b18d896 to
b817212
Compare
Problem
Every PDF ingestion fails in Marker with:
Marker downloads its rendering font lazily on the first conversion, defaulting to a path inside the installed package (
site-packages/static/fonts/).entrypoint.shdrops the app to a non-root user (APP_UID, GID 0), while the venv lives in the root-ownedopenrag_venvnamed volume whosesite-packagesisroot:root drwxr-xr-x. So the app user cannot createstatic/.Fix
Point
FONT_PATHat/app/data/fonts/, a bind-mounted directory the app user already owns (entrypoint.shgrants GID-0 write on/app/data). The ~15MB download happens once and persists across restarts.FONT_PATHis the only knob needed: marker'sSettingsis a pydanticBaseSettingswith no env prefix,FONT_DIRis dead except as the default forFONT_PATH, and the two other readers (providers/__init__.py,processors/debug.py) both go throughFONT_PATH.Why the extra anchor
Adding
environment:tox-openragalone silently does nothing for the GPU service: a service's ownenvironment:key replaces the merged one (YAML<<:is shallow), andopenragdeclares its own. The shared env is hoisted intox-openrag-envso both variants merge it explicitly, and the GPU service's list form is converted to mapping form to allow merging.Verification
On a live stack, after recreating the container:
task_state: COMPLETEDProcessed /tmp/tmpw2kxewk9.pdf in 33.75sPermissionErrorcount since restart: 0docker compose configconfirmsFONT_PATHresolves for bothopenragandopenrag-cpu, and theNVIDIA_*vars survive the list->mapping conversion🤖 Generated with Claude Code
Summary by CodeRabbit