deps: update shared (use new minio storage service) #944
Conversation
- handle bundle analysis changes: BundleChange requires bundle_name passed as kwarg now - remove service/storage.py as its no longer needed - change a bunch of imports from using `from shared.storage import ...` to `import ...` so that the mock_storage fixture just works
|
This PR includes changes to |
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #944 +/- ##
=======================================
Coverage 97.99% 97.99%
=======================================
Files 445 443 -2
Lines 35754 35721 -33
=======================================
- Hits 35036 35005 -31
+ Misses 718 716 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
✅ All tests successful. No failed tests were found. 📣 Thoughts on this report? Let Codecov know! | Powered by Codecov |
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. 📢 Thoughts on this report? Let us know! |
| @@ -234,7 +234,7 @@ def process_upload( | |||
| """ | |||
| commit_report: CommitReport = upload.report | |||
| repo_hash = ArchiveService.get_archive_hash(commit_report.commit.repository) | |||
| storage_service = get_storage_client() | |||
| storage_service = shared.storage.get_appropriate_storage_service(commit.repoid) | |||
| bundle_loader = BundleAnalysisReportLoader(storage_service, repo_hash) | |||
There was a problem hiding this comment.
I’m wondering why this is not using ArchiveService directly? As that is pretty much just storage_service + repo_hash
| @@ -16,7 +16,7 @@ def test_cache_test_rollups(self, mock_storage, transactional_db): | |||
| repo = RepositoryFactory() | |||
|
|
|||
| redis = get_redis_connection() | |||
| storage_service = get_storage_client() | |||
| storage_service = shared.storage.get_appropriate_storage_service() | |||
There was a problem hiding this comment.
I wonder whether its even possible to always pass the repo_id to all the calls to get_appropriate_storage_service?
In this very example, you are passing the repo_id in the code, but not in the test, so these are out of sync. For tests, this does not matter at all. But for other parts of the codebase, I believe it might matter quite a bit if the reader and writer side are out of sync and get a different storage service because of it.
make sure that we pass repoid to get_appropriate_storage_service whenever possible
passed as kwarg now
from shared.storage import ...to
import ...so that the mock_storage fixture just works