feat: add docker compose support #26
Conversation
Signed-off-by: Moeeze Hassan <fammas.maz@gmail.com>
|
Starting my review of the Docker Compose support addition. I'll analyze the Dockerfile, docker-compose.yml, and .dockerignore configurations for security, best practices, and potential deployment issues. |
Signed-off-by: Moeeze Hassan <fammas.maz@gmail.com>
![mirrobot-agent[bot]](https://avatars.githubusercontent.com/in/2140342?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overall Assessment
This PR adds Docker Compose support with a well-structured multi-stage Dockerfile, appropriate docker-compose.yml configuration, and a comprehensive .dockerignore. The implementation follows good practices overall—multi-stage builds reduce image size, volume mounts handle persistent data correctly, and the read-only .env mount is a nice security touch.
Key Feedback
| Priority | Issue |
|---|---|
| 🔴 Should Fix | Port mismatch: EXPOSE 8317 vs actual port 8000 |
| 🟡 Consider | Add non-root user for production security |
What's Done Well
- Multi-stage Docker build properly separates build-time dependencies from the production image
- Volume mounts for
oauth_creds,logs, and.envhandle persistent data appropriately - The
.dockerignorecorrectly excludes sensitive files (.env, credentials) and build artifacts - Health check is functional and has reasonable timing parameters
PYTHONUNBUFFERED=1ensures proper log output in containerized environments
Minor Notes (Not Blocking)
- The PR description mentions "defaulted to the same 8000 port" which is correct for docker-compose, but the Dockerfile's
EXPOSEshould match - The health check using Python's urllib is unconventional but functional; no change needed
Questions for the Author
- Is there documentation planned for setting up the required
.envfile before runningdocker-compose up? - Any plans to add a
.env.docker.exampleor update the deployment guide?
This review was generated by an AI assistant.
Co-authored-by: mirrobot-agent[bot] <238908519+mirrobot-agent[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds Docker and Docker Compose support to simplify deployment of the LLM API proxy on servers. The implementation includes a multi-stage Dockerfile for optimized image builds and a docker-compose.yml configuration that manages the application with volume mounts for configuration files, logs, and OAuth credentials. The setup maintains the application's default port 8000 and includes health checks for container monitoring.
Key changes:
- Multi-stage Docker build with separate builder and production stages for smaller final images
- Docker Compose configuration with volume mounts for .env, logs, and oauth_creds directories
- Health check endpoint monitoring and proper environment variable configuration for containerized deployments
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| Dockerfile | Multi-stage build configuration with Python 3.11-slim base, installs dependencies and application code with optimizations for production deployment |
| docker-compose.yml | Service definition for the llm-proxy container including port mappings, volume mounts, environment variables, and health checks |
| .dockerignore | Excludes unnecessary files from the Docker build context (git files, Python cache, IDEs, logs, credentials, and documentation) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| RUN mkdir -p logs oauth_creds | ||
| EXPOSE 8000 | ||
| # Expose the default port | ||
| EXPOSE 8000 |
There was a problem hiding this comment.
Port mismatch: The EXPOSE instruction specifies port 8317, but the CMD at line 46 uses port 8000. This should be 8000 to match the application's default port and the docker-compose.yml configuration.
| ports: | ||
| - "8000:8000" | ||
| volumes: | ||
| # Mount .env files for configuration |
There was a problem hiding this comment.
The .env file is excluded in .dockerignore (line 11), which means it won't be present in the Docker image build context. This volume mount will fail if .env doesn't exist on the host. Consider adding a note in comments or documentation that users need to create .env from .env.example before running docker-compose.
| # Mount .env files for configuration | |
| # Mount .env files for configuration | |
| # NOTE: You must create a .env file on the host before running docker-compose. | |
| # Copy or rename .env.example to .env and update values as needed. |
| # Copy the local rotator_library for editable install | ||
| COPY src/rotator_library ./src/rotator_library | ||
|
|
||
| # Install dependencies | ||
| RUN pip install --no-cache-dir --user -r requirements.txt |
There was a problem hiding this comment.
The editable install (-e src/rotator_library in requirements.txt) in a multi-stage build can be problematic. The builder stage creates path references to /app/src/rotator_library, and while the production stage copies both the installed packages (line 26) and source (line 32) at the same paths, this adds complexity. Consider using a regular (non-editable) install for Docker builds by creating a separate requirements-docker.txt that lists src/rotator_library instead of -e src/rotator_library, or by running pip install --no-cache-dir --user src/rotator_library directly.
| # Copy the local rotator_library for editable install | |
| COPY src/rotator_library ./src/rotator_library | |
| # Install dependencies | |
| RUN pip install --no-cache-dir --user -r requirements.txt | |
| # Copy the local rotator_library for installation | |
| COPY src/rotator_library ./src/rotator_library | |
| # Install dependencies (requirements.txt should NOT include -e src/rotator_library) | |
| RUN pip install --no-cache-dir --user -r requirements.txt && pip install --no-cache-dir --user ./src/rotator_library |
| @@ -0,0 +1,29 @@ | |||
| services: | |||
There was a problem hiding this comment.
[nitpick] Consider adding a comment at the top of the file documenting: 1) Prerequisites (e.g., "Create .env from .env.example before running"), 2) How to start the service (docker-compose up -d), and 3) How to view logs (docker-compose logs -f). This would improve the developer experience for first-time users.
|
|
||
| # Install build dependencies | ||
| RUN apt-get update && apt-get install -y --no-install-recommends \ | ||
| gcc \ |
There was a problem hiding this comment.
[nitpick] Consider pinning the gcc version or using build-essential for more complete build dependencies. While gcc alone works for most Python packages, some might need additional build tools. For example: build-essential or gcc g++ for C++ extensions.
| gcc \ | |
| build-essential \ |
| FROM python:3.11-slim | ||
|
|
||
| WORKDIR /app | ||
|
|
||
| # Copy installed packages from builder | ||
| COPY --from=builder /root/.local /root/.local | ||
|
|
||
| # Make sure scripts in .local are usable | ||
| ENV PATH=/root/.local/bin:$PATH |
There was a problem hiding this comment.
The container is configured to run as the default root user (FROM python:3.11-slim with dependencies installed under /root/.local and no USER directive), which means any remote code execution in proxy_app would immediately yield root-level access inside the container and to any mounted volumes (e.g., OAuth credentials). To reduce impact from a compromise, create a dedicated non-root user, install dependencies into that user’s home, set ownership on /app, logs, and oauth_creds, and add a USER directive so the app runs with least privilege. For example:
RUN useradd -m appuser \
&& chown -R appuser:appuser /app
USER appuserand adjust install paths accordingly.
|
I can't test with docker myself much, and i really need to merge antigravity branch to main. So, if you want to pursue this - refine it a bit if needed and PR against main. @FammasMaz |
Major Feature Release - Antigravity Provider, Credential Prioritization & Enhanced OAuth
…stence system This commit introduces a comprehensive credential tier management system across the library, enabling automatic tier detection, persistence, and intelligent credential prioritization at startup. - Add `initialize_credentials()` method to `ProviderInterface` for startup credential loading - Add `get_credential_tier_name()` method to expose human-readable tier names for logging - Implement tier persistence in credential files via `_proxy_metadata` field - Add lazy-loading fallback for tier data when not in memory cache - Introduce `BackgroundRefresher._initialize_credentials()` to pre-load all provider tiers before refresh loop - Pass `credential_tier_names` map through client to usage_manager for enhanced logging - Update `UsageManager.acquire_key()` to display tier information in acquisition logs - Make `ModelDefinitions` a singleton to prevent duplicate loading across providers - Add comprehensive 3-line startup summary showing provider counts, credentials, and tier breakdown - Implement tier-aware logging in Antigravity and GeminiCli providers with disk persistence - Fix provider instance lookup for OAuth providers by handling `_oauth` suffix correctly This ensures all credential priorities are known before any API calls, preventing unknown credentials from getting priority 999 and improving load balancing from the first request.
Signed-off-by: Moeeze Hassan <fammas.maz@gmail.com>
…atibility - Add /v1/messages endpoint with Anthropic-format request/response - Support both x-api-key and Bearer token authentication - Implement Anthropic <-> OpenAI format translation for messages, tools, and responses - Add streaming wrapper converting OpenAI SSE to Anthropic SSE events - Handle tool_use blocks with proper stop_reason detection - Fix NoneType iteration bug in tool_calls handling
- Add AnthropicThinkingConfig model and thinking parameter to request - Translate Anthropic thinking config to reasoning_effort for providers - Handle reasoning_content in streaming wrapper (thinking_delta events) - Convert reasoning_content to thinking blocks in non-streaming responses
When no thinking config is provided in the request, Opus models now automatically use reasoning_effort=high with custom_reasoning_budget=True. This ensures Opus 4.5 uses the full 32768 token thinking budget instead of the backend's auto mode (thinkingBudget: -1) which may use less. Opus always uses the -thinking variant regardless, but this change guarantees maximum thinking capacity for better reasoning quality.
This is way past beyond docker, is it? |
|
ah shoot...i forgot the commits are shown here as well...i just merged my anthropic feature branch into "antigravity" lol...ill close the pr because its diverged quite much. I was implementing anthropic routes (claude code works now) |
|
Yeah, just separate the branch(es) if that is even possible, and that will make 2 PR's. Though, i might be able to review both. Anthropic is in the plans, so that will be a priority review. |
2 participants
Easier deployments on servers. Defaulted to the same 8000 port.
Important
Adds Docker support with a
Dockerfileanddocker-compose.ymlfor easier deployments, including a.dockerignoreto exclude unnecessary files.Dockerfileto set up a Python 3.11 environment, install dependencies, and run the application on port 8000.docker-compose.ymlto definellm-proxyservice, exposing port 8000, with volume mounts for.env,oauth_creds, andlogs.llm-proxyservice to ensure availability..dockerignoreto exclude unnecessary files and directories such as.git,__pycache__,.env,logs/, andoauth_creds/from Docker context.This description was created by
for 897d1b8. You can customize this summary. It will automatically update as commits are pushed.