Skip to content
This repository was archived by the owner on May 5, 2025. It is now read-only.

fix: improve new minio implementation #546

Merged
merged 3 commits into from
Mar 3, 2025
Merged

Conversation

joseph-sentry
Copy link
Contributor

The team mentioned wanting to ship the new read implementation first
then the new write implementation so this commit enables that.

i'm folding the NewMinioStorageService functionality back into the
MinioStorageService and creating 2 new feature flags one for read and
one for write.

Based on the values of those feature flags the MinioStorageService
will choose an implementation of read/write.

I also removed the old USE_NEW_MINIO feature flag, and modified the
get_appropriate_storage_service function to make it so it no longer
caches the MinioStorageService and instead the MinioStorageService is
caching its internal minio_client since that is the expensive part
of creating a new MinioStorageService instance.

I also moved some code around

Copy link

codecov bot commented Feb 27, 2025

Codecov Report

Attention: Patch coverage is 84.48276% with 27 lines in your changes missing coverage. Please review.

Project coverage is 88.60%. Comparing base (bc3dd8d) to head (2161274).
Report is 2 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
shared/storage/write.py 80.32% 7 Missing and 5 partials ⚠️
shared/storage/compression.py 62.50% 8 Missing and 1 partial ⚠️
shared/storage/read.py 88.00% 4 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #546      +/-   ##
==========================================
+ Coverage   88.50%   88.60%   +0.09%     
==========================================
  Files         451      453       +2     
  Lines       13107    13044      -63     
  Branches     1528     1514      -14     
==========================================
- Hits        11600    11557      -43     
+ Misses       1184     1168      -16     
+ Partials      323      319       -4     
Flag Coverage Δ
shared-docker-uploader 88.60% <84.48%> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@codecov-notifications
Copy link

codecov-notifications bot commented Feb 27, 2025

Codecov Report

Attention: Patch coverage is 84.48276% with 27 lines in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
shared/storage/write.py 80.32% 7 Missing and 5 partials ⚠️
shared/storage/compression.py 62.50% 8 Missing and 1 partial ⚠️
shared/storage/read.py 88.00% 4 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

Copy link

codspeed-hq bot commented Feb 27, 2025

CodSpeed Performance Report

Merging #546 will create unknown performance changes

Comparing joseph/improve-new-minio (2161274) with main (bc3dd8d)

Summary

⚠️ No benchmarks were detected in both the base of the PR and the PR.\

@joseph-sentry joseph-sentry requested a review from a team February 27, 2025 14:30
@joseph-sentry joseph-sentry force-pushed the joseph/improve-new-minio branch from 57ea96b to 87f3a30 Compare February 27, 2025 17:03
@joseph-sentry joseph-sentry force-pushed the joseph/improve-new-minio branch 2 times, most recently from e091208 to 1dbf739 Compare February 28, 2025 14:49
this rollout is done and we are 100% enabled on prod so we are safe to
remove this and simplify the logic

starting off by removing all references to use_minio in shared
The team mentioned wanting to ship the new read implementation first
then the new write implementation so this commit enables that.

i'm folding the NewMinioStorageService functionality back into the
MinioStorageService and creating 2 new feature flags one for read and
one for write.

Based on the values of those feature flags the MinioStorageService
will choose an implementation of read/write.

I also removed the old USE_NEW_MINIO feature flag, and modified the
get_appropriate_storage_service function to make it so it no longer
caches the MinioStorageService and instead the MinioStorageService is
caching its internal minio_client since that is the expensive part
of creating a new MinioStorageService instance.

I also moved some code around
@joseph-sentry joseph-sentry force-pushed the joseph/improve-new-minio branch from 1dbf739 to 2161274 Compare February 28, 2025 15:26
@joseph-sentry joseph-sentry added this pull request to the merge queue Mar 3, 2025
Merged via the queue into main with commit 1d40dfa Mar 3, 2025
10 of 12 checks passed
@joseph-sentry joseph-sentry deleted the joseph/improve-new-minio branch March 3, 2025 15:22
@JerrySentry JerrySentry restored the joseph/improve-new-minio branch March 11, 2025 20:48
JerrySentry added a commit that referenced this pull request Mar 11, 2025
Copy link

overwatch-beta bot commented Mar 11, 2025

✅ Sentry found no issues in your recent changes ✅

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants