-
Notifications
You must be signed in to change notification settings - Fork 15
Switch to official minio container for CI #120
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
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 pull request switches from the unofficial Bitnami MinIO container to the official MinIO container in the CI workflow to improve reliability and standardization.
- Replaced the Bitnami MinIO service configuration with a Docker run command using the official MinIO image
- Updated test setup to create S3 buckets dynamically and added proper cleanup
- Switched from test context background to test-specific context for better cancellation handling
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
.github/workflows/ci.yml | Replaced Bitnami MinIO service with official MinIO container using docker run command |
archives/archives_test.go | Added dynamic bucket creation, test cleanup, and proper context usage |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
if s3Client.Test(ctx, "temba-archives") != nil { | ||
_, err = s3Client.Client.CreateBucket(ctx, &s3.CreateBucketInput{Bucket: aws.String("temba-archives")}) | ||
require.NoError(t, err) | ||
} |
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 bucket name 'temba-archives' is hardcoded in multiple places. Consider defining it as a constant to improve maintainability and reduce the risk of typos.
Copilot uses AI. Check for mistakes.
--health-interval 10s \ | ||
--health-timeout 5s \ | ||
--health-retries 5 \ | ||
minio/minio:latest minio server /data --console-address ":9001" |
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.
Using 'latest' tag for the MinIO container can lead to unpredictable CI builds when new versions are released. Consider pinning to a specific version tag for reproducible builds.
minio/minio:latest minio server /data --console-address ":9001" | |
minio/minio:RELEASE.2024-05-10T03-52-56Z minio server /data --console-address ":9001" |
Copilot uses AI. Check for mistakes.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #120 +/- ##
==========================================
- Coverage 59.16% 59.11% -0.05%
==========================================
Files 7 7
Lines 1310 1311 +1
==========================================
Hits 775 775
- Misses 429 430 +1
Partials 106 106 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
No description provided.