-
-
Notifications
You must be signed in to change notification settings - Fork 9
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
Add a prefix to bucket name #382
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #382 +/- ##
=======================================
Coverage 84.83% 84.83%
=======================================
Files 35 35
Lines 996 996
=======================================
Hits 845 845
Misses 151 151
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Thanks! Agree with the fix, but for the sake of limiting the number of env vars, I think we should mutualize this with SERVER_NAME
and probably renaming it to INSTANCE_NAME
(or by default using socket.gethostname()
or something unique to the VM so that we don't have to figure out which prefix isn't being used)
ok ! I just merged with main and pushed the modification |
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.
thanks! Only one thing to consider: the s3 bucket initialization might be conflicting with this new rule https://github.com/pyronear/pyro-api/blob/main/scripts/localstack/setup-s3.sh
Make sure to test it out before merging 🙏
Currently we have a conflict between bucket name because in our OVH account we have many environment (production + dev *2). This small refactor prevent this conflicts