Skip to content

Extending data_manager #2196

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

Merged
merged 16 commits into from
Mar 6, 2021
Merged

Conversation

GitHK
Copy link
Contributor

@GitHK GitHK commented Mar 5, 2021

What do these changes do?

Adds is_file_present_in_storage call to the data_manager. This will help with saving and restoring states for dynamic service.

Related issue/s

helps solving ITISFoundation/osparc-issues#429

How to test

Checklist

@codecov
Copy link

codecov bot commented Mar 5, 2021

Codecov Report

Merging #2196 (3a640c7) into master (b895660) will increase coverage by 0.4%.
The diff coverage is 92.3%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #2196     +/-   ##
========================================
+ Coverage    72.6%   73.1%   +0.4%     
========================================
  Files         465     465             
  Lines       17886   17899     +13     
  Branches     1763    1763             
========================================
+ Hits        12990   13085     +95     
+ Misses       4435    4355     -80     
+ Partials      461     459      -2     
Flag Coverage Δ
integrationtests 65.3% <84.6%> (+<0.1%) ⬆️
unittests 66.5% <15.3%> (-1.2%) ⬇️

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

Impacted Files Coverage Δ
...core-sdk/src/simcore_sdk/node_data/data_manager.py 93.3% <50.0%> (-2.1%) ⬇️
...core-sdk/src/simcore_sdk/node_ports/filemanager.py 82.8% <100.0%> (+1.2%) ⬆️
.../director/src/simcore_service_director/producer.py 61.1% <0.0%> (+0.4%) ⬆️
...simcore_service_webserver/projects/projects_api.py 87.3% <0.0%> (+0.9%) ⬆️
.../sidecar/src/simcore_service_sidecar/log_parser.py 82.0% <0.0%> (+8.9%) ⬆️
...vices/sidecar/src/simcore_service_sidecar/utils.py 79.4% <0.0%> (+24.6%) ⬆️
...sidecar/src/simcore_service_sidecar/celery_task.py 28.8% <0.0%> (+28.8%) ⬆️
...src/simcore_service_sidecar/celery_configurator.py 91.1% <0.0%> (+91.1%) ⬆️

@GitHK GitHK self-assigned this Mar 5, 2021
@GitHK GitHK added the t:enhancement Improvement or request on an existing feature label Mar 5, 2021
@GitHK GitHK modified the milestone: The White Rabbit Mar 5, 2021
@GitHK GitHK requested review from sanderegg and pcrespov March 5, 2021 13:27
@GitHK GitHK marked this pull request as ready for review March 5, 2021 13:27
@@ -297,3 +297,25 @@ async def upload_file(
return store_id, e_tag

raise exceptions.S3InvalidPathError(s3_object)


async def is_metadata_for_entry(store_id: str, s3_object: str) -> bool:
Copy link
Member

Choose a reason for hiding this comment

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

Q: i wonder if this coroutine can be cancelled because you are capturing with a broad-except. See this example:

async def foo():
  ...
  await is_metadata_for_entry(..)
  ...
  
# somewhere else, a code calls to cancel task associated to foo
task_foo.cancel()
await task_foo  

Copy link
Member

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

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

maybe just check my suggestions. otherwise this looks perfect.

@GitHK GitHK merged commit 0190de4 into ITISFoundation:master Mar 6, 2021
@sanderegg sanderegg added this to the The Red Panda milestone Mar 24, 2021
@sanderegg sanderegg mentioned this pull request Mar 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t:enhancement Improvement or request on an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants