♻️Storage: Code refactoring prior to changes (🚨🚨) #7088
Conversation
sanderegg
added
a:storage
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7088 +/- ##
==========================================
- Coverage 87.70% 87.48% -0.23%
==========================================
Files 1635 1638 +3
Lines 63892 63907 +15
Branches 1173 1087 -86
==========================================
- Hits 56037 55906 -131
- Misses 7545 7690 +145
- Partials 310 311 +1
Continue to review full report in Codecov by Sentry.
|
sanderegg
force-pushed
the
storage/refactoring
branch
6 times, most recently
from
2eaa872 to
dd968f9
Compare
sanderegg
force-pushed
the
storage/refactoring
branch
2 times, most recently
from
65be5bc to
2c7fcea
Compare
sanderegg
requested review from
pcrespov,
GitHK,
matusdrobuliak66 and
giancarloromeo
as code owners
sanderegg
changed the title
packages/service-library/src/servicelib/fastapi/long_running_tasks/client.py
Show resolved
Hide resolved
| "/locations/{location_id}/files/{file_id:path}", | ||
| response_model=Envelope[FileUploadResponseV1] | Envelope[FileUploadSchema], | ||
| ) | ||
| async def upload_file( |
There was a problem hiding this comment.
maybe this should also have cancel on disconnect decorator?
There was a problem hiding this comment.
good point. will add a middleware actually.
| @@ -56,10 +55,12 @@ def location_name() -> str: | |||
| return SimcoreS3DataManager.get_location_name() | |||
|
|
|||
|
|
|||
| @pytest.mark.skip(reason="legacy service") | |||
There was a problem hiding this comment.
Q: could you ellaborate more why this is disabled, I did not quite get the reason. Maybe it would be helpful to have a note here for the future.
I find these short reasons not that suseful in skipped tests
There was a problem hiding this comment.
actually I will re-enable it. I need to find a way to do this as this uses the aiohttp client.
I will probably have to start the real storage in order to run that
sanderegg
force-pushed
the
storage/refactoring
branch
from
182cdc6 to
0c07e27
Compare
|
What do these changes do?
Very noisy PR that refactors storage service from aiohttp- to fastapi-based service.
Important note: @bisgaard-itis @GitHK @mrnicegyu11 @odeimaiz @matusdrobuliak66 storage cannot be run as several replicas as I could not find a way to get the IP of the server via call to request during uploads of files. That means this will need a subsequent PR that will keep the state of the upload in the DB or call into AWS S3 until the completion of the S3 multipart is done. There will be a follow up on that in 1 week.
Related issue/s
storage#6887 by removing aiohttpHow to test
Dev-ops checklist