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

🐛 Fixes FileLink validation legacy format #3114

Merged
merged 7 commits into from
Jun 19, 2022

Conversation

pcrespov
Copy link
Member

@pcrespov pcrespov commented Jun 17, 2022

What do these changes do?

isolve-gpu e2e test failed (see incident) because comp_task tables included some outputs with an old FileLink format.

This PR adds a validator in FileLink that handles that legacy format.

A question remains, though: which service is still adding store="0" to the comp_task??

How to test

  • model_library tests adds example of data with legacy-format
  • simcore_sdk tests adds test with data taken from e2e test mentioned above

@pcrespov pcrespov self-assigned this Jun 17, 2022
@codecov
Copy link

codecov bot commented Jun 17, 2022

Codecov Report

Merging #3114 (9c68406) into master (0b1b17b) will increase coverage by 1.4%.
The diff coverage is 100.0%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #3114     +/-   ##
========================================
+ Coverage    79.6%   81.0%   +1.4%     
========================================
  Files         706     706             
  Lines       30451   30457      +6     
  Branches     3944    3945      +1     
========================================
+ Hits        24245   24677    +432     
+ Misses       5332    4936    -396     
+ Partials      874     844     -30     
Flag Coverage Δ
integrationtests 65.9% <ø> (+<0.1%) ⬆️
unittests 77.2% <100.0%> (+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/node_ports_v2/port.py 80.6% <ø> (ø)
...ls-library/src/models_library/projects_nodes_io.py 97.2% <100.0%> (+0.2%) ⬆️
...ector_v2/modules/comp_scheduler/background_task.py 83.3% <0.0%> (-8.4%) ⬇️
...ore_service_director_v2/utils/client_decorators.py 73.3% <0.0%> (-3.4%) ⬇️
.../simcore_service_catalog/db/repositories/groups.py 72.9% <0.0%> (-2.8%) ⬇️
.../director/src/simcore_service_director/producer.py 62.1% <0.0%> (+0.2%) ⬆️
...tor_v2/modules/dynamic_sidecar/scheduler/events.py 92.3% <0.0%> (+1.0%) ⬆️
...vice_webserver/exporter/formatters/formatter_v1.py 79.0% <0.0%> (+1.1%) ⬆️
...imcore_service_webserver/garbage_collector_core.py 70.1% <0.0%> (+1.8%) ⬆️
...rector_v2/modules/comp_scheduler/dask_scheduler.py 86.5% <0.0%> (+1.9%) ⬆️
... and 38 more

@pcrespov pcrespov changed the title 🐛 Fix/file link validation 🐛 Fixes FileLink validation legacy format Jun 17, 2022
@pcrespov pcrespov marked this pull request as ready for review June 17, 2022 12:51
@pcrespov pcrespov requested review from GitHK and sanderegg as code owners June 17, 2022 12:51
@pcrespov pcrespov requested review from mguidon and odeimaiz June 17, 2022 12:54
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.

👍

@pcrespov pcrespov force-pushed the fix/file-link-validation branch from ee41e1f to 9c68406 Compare June 17, 2022 13:26
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.5% 0.5% Duplication

@pcrespov pcrespov added this to the Diolkos milestone Jun 17, 2022
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.

good one. Thanks a lot!

@pcrespov pcrespov merged commit d649b5e into ITISFoundation:master Jun 19, 2022
@pcrespov pcrespov deleted the fix/file-link-validation branch June 19, 2022 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants