feat: dockerize pixelrag serve engine - #136
Youcef3939 wants to merge 1 commit into
Conversation
|
@Youcef3939 is attempting to deploy a commit to the andylizf's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
yo @andylizf, if you have some time can you kindly review the pr? |
andylizf
left a comment
There was a problem hiding this comment.
Thanks for this, and for pinging @jenilbanavani on #110 before you started — that saved a duplicate.
I went through the three files against the current tree. The build itself looks right: the env var names match the argparse defaults in serve/src/pixelrag_serve/api.py, the COPY list matches the packages list under [tool.hatch.build.targets.wheel], and pulling /root/.cache/pixelrag/chrome out of the builder matches INSTALL_DIR in render/src/pixelrag_render/chrome.py. Two things block a merge, then some smaller ones.
docker compose up won't start. Both volumes are named volumes, so on a fresh machine they come up empty, and pixelrag-index is read-only so nothing can ever populate it. The server calls open(args.articles_json) unguarded at api.py:681 and dies with FileNotFoundError on /data/index/articles.json before it serves a single request. Bind mounts would work here:
volumes:
- ./index:/data/index:ro
- ./tiles:/data/tilesThat needs a README line too: articles.json is written by pixelrag index (see index/src/pixelrag_index/pipelines.py), and nothing in the README currently tells a new user that the index directory has to exist before serve will come up.
The build fails on arm64. install_chrome() raises for anything that isn't linux-x86_64, so RUN uv run pixelshot install-chrome is a hard failure on an Apple Silicon machine. Either skip that step when the target platform isn't x86_64, or document --platform linux/amd64 and say why it's needed.
Smaller items:
- The
serveextra resolves torch and torchvision through the cu129 index ([tool.uv.sources]), so the image carries the whole CUDA stack. I haven't built it, but those wheels alone run to several GB. The compose service reserves no GPU, so as written it runs CPU inference on a CUDA build. Either add a GPU reservation or say in the README that the image targets a GPU host. Qwen/Qwen3-VL-Embedding-2Bis fetched from HuggingFace at startup with no cache volume, so every recreated container re-downloads it.HF_HOME=/data/hfplus a volume for it would fix that.- The runtime stage runs as root. Worth adding a non-root user for something that exposes a port.
- All three files are missing a trailing newline.
One question on positioning, because it changes what the README should say. Production runs on bare metal under systemd with blue-green slots (deploy/README.md), and that isn't changing. I'd rather this land as the local and self-host path, documented so nobody reads it as a replacement for deploy/. A short "Run with Docker" section in the root README covering build, the two directories to mount, and the amd64 caveat would close out #110 properly.
Separately: the failing Vercel check is a fork deploy waiting on team authorization from me, not anything in your branch. Ignore it.
Once the compose volumes and the arm64 build are sorted I'll take another pass.
this pull request introduces docker support for the project, enabling containerized development and deployment. the main changes include adding a multi-stage
Dockerfilefor building and running the application, a.dockerignorefile to optimize build context, and adocker-compose.ymlfor orchestrating the service and persistent volumesdocker support and build optimization:
Dockerfileto build and run the application, installing all necessary system and python dependencies, copying source files, and setting up the runtime environment.dockerignorefile to exclude unnecessary files and directories from the docker build context, improving build performance and reducing image sizeservice orchestration:
docker-compose.ymlfile to define thepixelrag-apiservice, expose the application on port 8000, set environment variables, and mount persistent volumes for index and tiles dataissue #110