Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the CUDA and CUDNN configuration for GPU support, makes CUDNN version configurable, updates HDF5 library package names, and adds a production .env configuration file. However, the PR contains several critical issues including committed production credentials and version inconsistencies.
- CUDA version updated from 12.3.0 to 12.3.2 across Dockerfile.gpu, docker-compose.production.yml, and .env.production.example
- CUDNN version made configurable via CUDNN_VERSION build argument
- HDF5 library package names changed from libhdf5-310/libhdf5-hl-310 to libhdf5-103-1/libhdf5-hl-100 with fallback logic
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| docker-compose.production.yml | Updated CUDA version to 12.3.2 and added CUDNN_VERSION build argument for both writebot and celery-worker services |
| Dockerfile.gpu | Updated CUDA to 12.3.2, made CUDNN version configurable, changed HDF5 package names, and added fallback installation logic |
| .env.production.example | Updated CUDA version to 12.3.2 and added CUDNN_VERSION=9 configuration |
| .env | CRITICAL: Adds production .env file with actual credentials, secrets, and internal IP addresses that should not be committed |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| libhdf5-103-1 \ | ||
| libhdf5-hl-100 \ |
There was a problem hiding this comment.
The libhdf5 package version numbers have changed from libhdf5-310/libhdf5-hl-310 to libhdf5-103-1/libhdf5-hl-100, which appears to be a downgrade (from 3.10.x to 1.03.x/1.00.x based on the package naming). This is inconsistent and suggests the package names may not be correct for Ubuntu 22.04. Verify these are the correct package names for the target Ubuntu version. For Ubuntu 22.04, you may want to check the actual available package versions with "apt-cache search libhdf5" on the target system before committing these changes.
| && rm -rf /var/lib/apt/lists/* \ | ||
| || (apt-get update && apt-get install -y \ | ||
| python${PYTHON_VERSION} \ | ||
| python3-pip \ |
There was a problem hiding this comment.
The fallback logic in this apt-get command has a potential issue. If the first installation attempt fails (e.g., libhdf5-103-1 or libhdf5-hl-100 are not available), the fallback only installs Python and pip, omitting the HDF5 libraries entirely. This could lead to runtime errors when the application tries to use HDF5 functionality. Consider either:
- Using a fallback that still attempts to install alternative HDF5 package versions
- Failing the build explicitly if HDF5 libraries cannot be installed, rather than silently continuing without them
This is especially important since these package names changed from libhdf5-310 and libhdf5-hl-310, suggesting package availability may vary across Ubuntu versions.
| python3-pip \ | |
| python3-pip \ | |
| libhdf5-dev \ |
1 participant
No description provided.