Add service image build performance improvements#647
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe CI workflow was refactored to an include-based matrix that maps services to paths and detects path changes to gate execution. AWS credentials, ECR login, Docker Buildx, Flox installation, artifact download, and the unified docker/build-push-action are conditionally executed only when a service changed or on scheduled runs. The build step now fetches the AWS account ID for ECR tagging and uses registry caching. Pulumi deployment now includes an explicit stack initialization step. Dockerfiles were updated to add build cache mounts, SSL handling for datamanager, and removed explicit cache-busting flags for Python/Rust package syncs. Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Path as Path Change Detector
participant AWS as AWS (STS/ECR)
participant Buildx as Docker Buildx
participant Registry as ECR Registry
participant Flox as Flox Installer
participant Artifact as Artifact Storage
participant Pulumi as Pulumi
GH->>Path: determine changed services / global changes
alt service_changed or schedule
Path-->>GH: service list (e.g., equitypricemodel)
GH->>AWS: Configure AWS credentials
AWS-->>GH: creds set
GH->>AWS: Get AWS Account ID
AWS-->>GH: account id (for ECR path)
GH->>Buildx: Setup Docker Buildx (conditional)
GH->>Flox: Install Flox (conditional)
alt artifact needed (equitypricemodel)
GH->>Artifact: Download artifact
Artifact-->>GH: artifact files
end
GH->>Buildx: Build and push via docker/build-push-action
Buildx->>Registry: Push images (tags: latest, ECR path) with cache
Registry-->>Buildx: push result
GH->>Pulumi: pulumi stack init
Pulumi-->>GH: stack ready
GH->>Pulumi: pulumi up (deploy)
Pulumi-->>GH: deployment result
else no_relevant_changes
Path-->>GH: no service changed
GH-->>GH: skip AWS/Docker/Pulumi/Flox steps
end
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (3)📚 Learning: 2025-12-23T16:45:01.573ZApplied to files:
📚 Learning: 2025-12-23T16:45:01.573ZApplied to files:
📚 Learning: 2025-12-23T16:45:01.573ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (7)
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.
Pull request overview
This PR focuses on optimizing Docker build performance for service images and the CI/CD pipeline. The changes remove unnecessary cache-disabling flags from Python builds and implement proper build caching for Rust builds, while also adding smart change detection to the GitHub workflow to avoid unnecessary builds.
Key Changes:
- Removed
--no-cacheflags from Python dependency installation to enable package caching - Implemented Docker BuildKit mount caching for Rust builds to cache dependencies and build artifacts
- Added path-based change detection to skip builds when services haven't changed
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| applications/portfoliomanager/Dockerfile | Removed --no-cache flag from uv sync to enable Python package caching |
| applications/equitypricemodel/Dockerfile | Removed --no-cache flag from uv sync to enable Python package caching |
| applications/datamanager/Dockerfile | Added BuildKit cache mounts for Cargo registry and build artifacts, with intermediate binary copy |
| .github/workflows/launch_infrastructure.yaml | Added path filtering for conditional builds, replaced custom mask commands with native Docker build-push action, and integrated registry-based layer caching |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/launch_infrastructure.yamlapplications/datamanager/Dockerfileapplications/equitypricemodel/Dockerfileapplications/portfoliomanager/Dockerfile
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run Rust code checks
🔇 Additional comments (8)
applications/portfoliomanager/Dockerfile (1)
13-13: LGTM! Caching enabled for improved build performance.Removing the
--no-cacheflag allows uv to cache downloaded packages, which should significantly improve build times on subsequent builds while maintaining correctness through Docker's layer caching mechanism.applications/datamanager/Dockerfile (3)
29-38: Excellent cache mount implementation for Rust builds.The BuildKit cache mounts for both the Cargo registry and target directory will significantly improve build times. The binary is correctly copied to
/tmpwithin the sameRUNcommand (line 38) before the cache mount is detached, ensuring it persists to the next stage.
53-57: LGTM! Binary copy and entrypoint are correctly configured.The binary copy from
/tmp/datamanager(staged during the build phase) and the explicit entrypoint are properly aligned with the cache mount strategy implemented in the builder stage.
42-49: SSL certificate setup is necessary for the datamanager service.The datamanager service uses the
reqwestHTTP client library to communicate with HTTPS endpoints, specifically the Massive API athttps://api.massive.io. The CA certificates and SSL environment variables (SSL_CERT_FILEandSSL_CERT_DIR) are required for TLS certificate verification during these HTTPS requests.applications/equitypricemodel/Dockerfile (1)
13-13: LGTM! Caching enabled for improved build performance.Removing the
--no-cacheflag allows uv to cache downloaded packages, consistent with the optimization applied to portfoliomanager. This should improve build times while maintaining correctness..github/workflows/launch_infrastructure.yaml (3)
22-28: Excellent matrix restructure for maintainability.The include-based matrix clearly ties each service to its path, making the configuration more maintainable and easier to extend with additional services.
32-43: Good path-based change detection implementation.The path filter correctly identifies changes to service-specific paths, shared Python libraries, and dependency lock files. The filter appropriately triggers builds when any of these dependencies change.
71-84: Strong improvement with Docker Buildx and registry caching.The migration to
docker/build-push-action@v5with ECR-based registry caching should significantly improve build times. Themode=maxcache setting ensures maximum layer reuse.
…tructure workflow
Overview
Changes
Comments
These were all generated by Claude and seem reasonable.
Summary by CodeRabbit
Chores
Bug Fixes / Reliability
✏️ Tip: You can customize this high-level summary in your review settings.