🐛 Concurrent S3 bucket creation attempt #8045
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #8045 +/- ##
===========================================
- Coverage 87.89% 68.19% -19.71%
===========================================
Files 1853 751 -1102
Lines 71495 35214 -36281
Branches 1258 176 -1082
===========================================
- Hits 62843 24015 -38828
- Misses 8287 11141 +2854
+ Partials 365 58 -307
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
giancarloromeo
changed the title
giancarloromeo
requested review from
pcrespov,
matusdrobuliak66,
bisgaard-itis and
GitHK
| if await client.bucket_exists( | ||
| bucket=settings.STORAGE_S3.S3_BUCKET_NAME | ||
| ): | ||
| _logger.info( | ||
| "S3 bucket %s exists already, skipping creation", | ||
| settings.STORAGE_S3.S3_BUCKET_NAME, | ||
| ) | ||
| break |
There was a problem hiding this comment.
this is not needed, why do you add it?
There was a problem hiding this comment.
FYI: the before_sleep_log is alreayd logging. If you want a different message per attempt, you can also customize it there. Check tenacity doc
log_context(
_logger, logging.DEBUG, msg="setup.s3_bucket.cleanup_ctx"
)
There was a problem hiding this comment.
@sanderegg well, the behavior seems clear: we want to ensure that S3 bucket has been created. Why not check if the bucket already exists before trying to blindly create it, that for 99% will end in an exception?
| if await client.bucket_exists( | ||
| bucket=settings.STORAGE_S3.S3_BUCKET_NAME | ||
| ): | ||
| _logger.info( | ||
| "S3 bucket %s exists already, skipping creation", | ||
| settings.STORAGE_S3.S3_BUCKET_NAME, | ||
| ) | ||
| break |
There was a problem hiding this comment.
FYI: the before_sleep_log is alreayd logging. If you want a different message per attempt, you can also customize it there. Check tenacity doc
log_context(
_logger, logging.DEBUG, msg="setup.s3_bucket.cleanup_ctx"
)
|
What do these changes do?
This PR makes the setup of S3 in storage more robust, especially when trying to create the Bucket.
It would be ideal to move the creation out of the service, in a script, and not in the service logic.
Related issue/s
sto-workerservice fails to start due to concurrent S3 bucket creation during startup #8043How to test
Dev-ops