-
Notifications
You must be signed in to change notification settings - Fork 63
docker arch refactor #224
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
docker arch refactor #224
Conversation
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.
Pull Request Overview
This PR refactors the Docker architecture and updates project dependencies, migrating from Python 3.13 to 3.14, pinning several dependencies to specific versions, and streamlining the Dockerfile build process.
Key Changes:
- Upgrade Python requirement from 3.13 to 3.14 across pyproject.toml and Dockerfile
- Pin critical dependencies (sqlalchemy, uvicorn, polars, rotoger) to specific versions instead of using ranges
- Improve Docker Compose service dependencies with health checks for better startup orchestration
Reviewed Changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| pyproject.toml | Version bump to 0.22.0, Python 3.14 requirement, and dependency pinning for sqlalchemy, uvicorn, polars, and rotoger |
| pyproject.old | Removal of legacy Poetry configuration file |
| compose.yml | Enhanced service dependencies with health check conditions for postgres and redis |
| Makefile | Updated docker-clean command to use piped grep/xargs for volume removal |
| Dockerfile | Refactored to use Python 3.14, renamed build stage to base, streamlined uv installation, and removed libxml2 dependency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| python3-setuptools \ | ||
| python3.13-dev \ | ||
| git | ||
| python3.14-dev |
Copilot
AI
Nov 13, 2025
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.
The git package has been removed from the build dependencies. If any Python packages need to be installed from git repositories (as was done in the old pyproject.old with pendulum), or if any dependencies use git during their build process, this will cause the build to fail. Verify that git is not needed before removing it.
| python3.14-dev | |
| python3.14-dev \ | |
| git |
| python3.14 \ | ||
| libpython3.14 \ | ||
| libpcre3 |
Copilot
AI
Nov 13, 2025
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.
The libxml2 dependency has been removed. If the application or any of its dependencies (such as lxml, which is commonly used) require libxml2, this will cause runtime errors. Verify that this dependency is truly unnecessary before removing it.
| "redis>=6.4.0", | ||
| "bcrypt>=4.3.0", | ||
| "polars[pyarrow]>=1.33.1", | ||
| "polars==1.35.2", |
Copilot
AI
Nov 13, 2025
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.
The [pyarrow] extra has been removed from the polars dependency. If the application uses PyArrow-backed operations in Polars (e.g., reading Parquet files, Arrow IPC, or certain data conversions), this may break functionality. Consider whether the pyarrow extra is still needed.
| "polars==1.35.2", | |
| "polars==1.35.2", | |
| "pyarrow>=14.0.0", |
|
|
||
|
|
||
| COPY --from=ghcr.io/astral-sh/uv:latest /uv /usr/local/bin/uv | ||
| COPY --from=ghcr.io/astral-sh/uv:latest /uv /uvx /bin/ |
Copilot
AI
Nov 13, 2025
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.
The copy destination path is incorrect. The destination should be /usr/local/bin/ or similar, not /bin/. Additionally, copying both /uv and /uvx separately suggests they should maintain their individual paths. The correct syntax should be COPY --from=ghcr.io/astral-sh/uv:latest /uv /uvx /usr/local/bin/ to copy both executables to the bin directory.
| COPY --from=ghcr.io/astral-sh/uv:latest /uv /uvx /bin/ | |
| COPY --from=ghcr.io/astral-sh/uv:latest /uv /uvx /usr/local/bin/ |
| UV_COMPILE_BYTECODE=1 \ | ||
| UV_PYTHON_DOWNLOADS=never \ | ||
| UV_PYTHON=python3.13 \ | ||
| UV_PYTHON=python3.14 \ |
Copilot
AI
Nov 13, 2025
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.
[nitpick] The UV_PYTHON_DOWNLOADS=never environment variable has been removed. This was likely intentional to allow uv to download Python if needed, but combined with the change to Python 3.14 (which may not be available), this could cause unexpected behavior. If the intention is to use only system Python, this should be kept as UV_PYTHON_DOWNLOADS=never.
| UV_PYTHON=python3.14 \ | |
| UV_PYTHON=python3.14 \ | |
| UV_PYTHON_DOWNLOADS=never \ |
| --locked \ | ||
| --no-dev \ | ||
| --no-install-project | ||
| RUN cd /_lock && uv sync --locked --no-install-project |
Copilot
AI
Nov 13, 2025
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.
The --no-dev flag has been removed from the uv sync command. This means development dependencies will now be installed in the production Docker image, increasing the image size unnecessarily. If this is a production image, add back --no-dev to exclude development dependencies.
| RUN cd /_lock && uv sync --locked --no-install-project | |
| RUN cd /_lock && uv sync --locked --no-install-project --no-dev |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
No description provided.