Skip to content
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

♻️ Replace auto-generated storage python client #2578

Conversation

sanderegg
Copy link
Member

@sanderegg sanderegg commented Oct 8, 2021

What do these changes do?

A long time ago we used the tool openapi-generator-cli for generating storage client code from its openapi specifications. This was useful at the time but since then we needed some specifc changes that made the generator cumbersome. Now the only remaining user of that client was node_ports package. in order to ease the migration to dask the client is now removed.

  • Removed auto-generated storage python client and all references to it from the codebase
  • Created light-weight client in node_ports package to communicate with storage API
  • Refactored auto-update of file_meta_data table from storage by leveraging tenacity
  • when getting a file info, if the file is not YET on storage, then it is not returned as it is invalid.

Related issue/s

How to test

make build
cd packages/simcore-sdk
make tests

Checklist

@sanderegg sanderegg added a:storage issue related to storage service a:pipeline-services issues on non-core services in user's pipeline (use osparc-services repo for specifics) a:director-v2 issue related with the director-v2 service a:dynamic-sidecar dynamic-sidecar service labels Oct 8, 2021
@sanderegg sanderegg self-assigned this Oct 8, 2021
@codecov
Copy link

codecov bot commented Oct 8, 2021

Codecov Report

Merging #2578 (a980ed7) into master (89fef0d) will increase coverage by 0.0%.
The diff coverage is 77.2%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #2578   +/-   ##
======================================
  Coverage    77.1%   77.1%           
======================================
  Files         616     619    +3     
  Lines       23862   23931   +69     
  Branches     2343    2358   +15     
======================================
+ Hits        18413   18471   +58     
- Misses       4821    4826    +5     
- Partials      628     634    +6     
Flag Coverage Δ
integrationtests 65.9% <74.8%> (-0.1%) ⬇️
unittests 71.6% <61.7%> (-0.1%) ⬇️

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

Impacted Files Coverage Δ
...simcore-sdk/src/simcore_sdk/config/http_clients.py 100.0% <ø> (ø)
...core-sdk/src/simcore_sdk/node_ports_v2/__init__.py 100.0% <ø> (ø)
.../src/simcore_service_api_server/modules/storage.py 52.6% <0.0%> (ø)
...catalog/src/simcore_service_catalog/core/events.py 57.5% <0.0%> (ø)
...es/storage/src/simcore_service_storage/handlers.py 71.3% <0.0%> (ø)
...vices/storage/src/simcore_service_storage/utils.py 92.1% <ø> (+3.2%) ⬆️
...g/src/simcore_service_catalog/services/director.py 42.0% <40.9%> (+0.2%) ⬆️
...rc/simcore_sdk/node_ports_common/storage_client.py 73.3% <73.3%> (ø)
...k/src/simcore_sdk/node_ports_common/filemanager.py 81.2% <77.7%> (+<0.1%) ⬆️
...ervices/storage/src/simcore_service_storage/dsm.py 76.2% <83.7%> (+0.9%) ⬆️
... and 8 more

@sanderegg sanderegg force-pushed the maintenance/replace_storage_python_client branch from 213fd20 to d90cc70 Compare October 17, 2021 19:44
@sanderegg sanderegg force-pushed the maintenance/replace_storage_python_client branch from 0773bc6 to 714096a Compare October 18, 2021 12:14
@sanderegg sanderegg changed the title WIP: ♻️ Replace storage python client ♻️ Replace auto-generated storage python client Oct 18, 2021
@sanderegg sanderegg added this to the Capra delle nevi milestone Oct 18, 2021
@sanderegg sanderegg marked this pull request as ready for review October 18, 2021 15:12
Copy link
Contributor

@GitHK GitHK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice! Some minor stuff.

Copy link
Member

@pcrespov pcrespov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@sanderegg sanderegg merged commit bac03b9 into ITISFoundation:master Oct 19, 2021
@sanderegg sanderegg deleted the maintenance/replace_storage_python_client branch October 19, 2021 21:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:director-v2 issue related with the director-v2 service a:dynamic-sidecar dynamic-sidecar service a:pipeline-services issues on non-core services in user's pipeline (use osparc-services repo for specifics) a:storage issue related to storage service
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants