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

♻️ Maintenance: cleanup catalog #3983

Merged
merged 21 commits into from
Mar 18, 2023

Conversation

sanderegg
Copy link
Member

@sanderegg sanderegg commented Mar 16, 2023

What do these changes do?

  • De-duplicate models.
  • /{service_key}/{service_version}/resources now checks again that the service passed really uses the correct expected names and versions like all other API entrypoints of the catalog service (@elisabettai this should help with your issue this morning as well)
  • remove caching in catalog of /resources entrypoint as this prevents changing the DB and having immediate response for no clear gain

Related issue/s

How to test

Checklist

@sanderegg sanderegg added t:maintenance Some planned maintenance work a:catalog catalog service a:models-library labels Mar 16, 2023
@sanderegg sanderegg added this to the Mithril milestone Mar 16, 2023
@sanderegg sanderegg self-assigned this Mar 16, 2023
@codecov
Copy link

codecov bot commented Mar 16, 2023

Codecov Report

Merging #3983 (f760f1d) into master (b2a526e) will decrease coverage by 0.4%.
The diff coverage is 75.8%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #3983     +/-   ##
========================================
- Coverage    86.1%   85.8%   -0.4%     
========================================
  Files         765     828     +63     
  Lines       34884   37194   +2310     
  Branches      848     487    -361     
========================================
+ Hits        30051   31919   +1868     
- Misses       4609    5147    +538     
+ Partials      224     128     -96     
Flag Coverage Δ
integrationtests 66.7% <ø> (+13.9%) ⬆️
unittests 82.3% <75.8%> (-1.9%) ⬇️

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

Impacted Files Coverage Δ
...s/models-library/src/models_library/basic_regex.py 100.0% <ø> (ø)
...imcore_service_catalog/db/repositories/products.py 71.4% <0.0%> (ø)
...catalog/src/simcore_service_catalog/utils/pools.py 0.0% <0.0%> (ø)
.../simcore_service_catalog/db/repositories/groups.py 59.4% <42.8%> (ø)
...rc/simcore_service_catalog/db/repositories/dags.py 45.9% <45.4%> (ø)
...c/simcore_service_catalog/core/background_tasks.py 59.1% <50.0%> (ø)
...imcore_service_catalog/db/repositories/services.py 74.3% <61.9%> (ø)
...log/src/simcore_service_catalog/api/routes/dags.py 62.7% <66.6%> (ø)
.../simcore_service_catalog/services/access_rights.py 85.1% <80.0%> (ø)
...e_service_catalog/api/routes/services_resources.py 86.6% <88.8%> (ø)
... and 18 more

... and 303 files with indirect coverage changes

@sanderegg sanderegg marked this pull request as ready for review March 16, 2023 21:08
Copy link
Collaborator

@elisabettai elisabettai left a comment

Choose a reason for hiding this comment

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

Thanks! You were too fast, I haven't had time to create an issue yet. 😉

@sanderegg sanderegg force-pushed the maintenance/cleanup_catalog branch from 6c3f96d to eef7480 Compare March 17, 2023 07:17
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.

👍
Question: are dynamic-services happy about these changes? Did you get to test with s4l and other muli container services?

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.

Thx for the cleanup!

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.

Thx for the cleanup!

@sanderegg sanderegg force-pushed the maintenance/cleanup_catalog branch from dec7db6 to f8df4d0 Compare March 17, 2023 16:14
@sanderegg sanderegg force-pushed the maintenance/cleanup_catalog branch from f8df4d0 to f760f1d Compare March 17, 2023 17:19
@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.0% 0.0% Duplication

@codeclimate
Copy link

codeclimate bot commented Mar 17, 2023

Code Climate has analyzed commit f760f1d and detected 0 issues on this pull request.

View more on Code Climate.

@sanderegg sanderegg merged commit 16d7cfa into ITISFoundation:master Mar 18, 2023
@sanderegg sanderegg deleted the maintenance/cleanup_catalog branch March 21, 2023 10:24
@pcrespov pcrespov mentioned this pull request Mar 23, 2023
31 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:catalog catalog service a:models-library t:maintenance Some planned maintenance work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

personalizable resource limits M1-12 Maintenance and DevOps
5 participants