Skip to content

♻️ Refactor catalog domain in webserver #7308

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

Conversation

bisgaard-itis
Copy link
Contributor

@bisgaard-itis bisgaard-itis commented Mar 5, 2025

What do these changes do?

  • Refactor Catalog domain of webserver so it now adheres to the new design @pcrespov
  • Improve code coverage to 94% in our unittests:
    image

Related issue/s

How to test

Dev-ops checklist

@bisgaard-itis bisgaard-itis marked this pull request as ready for review March 5, 2025 14:01
@bisgaard-itis bisgaard-itis self-assigned this Mar 5, 2025
@bisgaard-itis bisgaard-itis added a:webserver issue related to the webserver service a:catalog catalog service labels Mar 5, 2025
@bisgaard-itis bisgaard-itis added this to the The Awakening milestone Mar 5, 2025
Copy link

codecov bot commented Mar 5, 2025

Codecov Report

Attention: Patch coverage is 95.78947% with 4 lines in your changes missing coverage. Please review.

Project coverage is 84.06%. Comparing base (a4a3bc3) to head (9d55786).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7308      +/-   ##
==========================================
- Coverage   88.57%   84.06%   -4.51%     
==========================================
  Files        1527      664     -863     
  Lines       60106    31667   -28439     
  Branches      711      168     -543     
==========================================
- Hits        53241    26622   -26619     
+ Misses       6672     4987    -1685     
+ Partials      193       58     -135     
Flag Coverage Δ
integrationtests 65.39% <61.05%> (-0.01%) ⬇️
unittests 87.19% <95.78%> (-0.49%) ⬇️
Components Coverage Δ
api 76.84% <ø> (ø)
pkg_aws_library ∅ <ø> (∅)
pkg_dask_task_models_library ∅ <ø> (∅)
pkg_models_library ∅ <ø> (∅)
pkg_notifications_library ∅ <ø> (∅)
pkg_postgres_database ∅ <ø> (∅)
pkg_service_integration ∅ <ø> (∅)
pkg_service_library ∅ <ø> (∅)
pkg_settings_library ∅ <ø> (∅)
pkg_simcore_sdk 77.30% <ø> (-8.16%) ⬇️
agent ∅ <ø> (∅)
api_server ∅ <ø> (∅)
autoscaling ∅ <ø> (∅)
catalog ∅ <ø> (∅)
clusters_keeper ∅ <ø> (∅)
dask_sidecar ∅ <ø> (∅)
datcore_adapter ∅ <ø> (∅)
director ∅ <ø> (∅)
director_v2 78.54% <ø> (-12.76%) ⬇️
dynamic_scheduler ∅ <ø> (∅)
dynamic_sidecar 89.43% <ø> (-0.86%) ⬇️
efs_guardian ∅ <ø> (∅)
invitations ∅ <ø> (∅)
osparc_gateway_server ∅ <ø> (∅)
payments ∅ <ø> (∅)
resource_usage_tracker ∅ <ø> (∅)
storage ∅ <ø> (∅)
webclient ∅ <ø> (∅)
webserver 85.81% <95.78%> (+0.48%) ⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a4a3bc3...9d55786. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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! left some suggestions for your considerations

@pcrespov
Copy link
Member

pcrespov commented Mar 5, 2025

BTW, did you figure out the issue with coverage? Checkout what is the current state in the api-keys domain
https://app.codecov.io/gh/ITISFoundation/osparc-simcore/tree/master/services%2Fweb%2Fserver%2Fsrc%2Fsimcore_service_webserver%2Fapi_keys

@bisgaard-itis bisgaard-itis requested a review from pcrespov March 6, 2025 06:33
Copy link
Collaborator

@matusdrobuliak66 matusdrobuliak66 left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

Copy link

sonarqubecloud bot commented Mar 6, 2025

@bisgaard-itis bisgaard-itis merged commit 427d751 into ITISFoundation:master Mar 6, 2025
92 of 96 checks passed
@bisgaard-itis bisgaard-itis deleted the refactor-catalog-domain-in-webserver branch March 6, 2025 13:07
@bisgaard-itis
Copy link
Contributor Author

BTW, did you figure out the issue with coverage? Checkout what is the current state in the api-keys domain https://app.codecov.io/gh/ITISFoundation/osparc-simcore/tree/master/services%2Fweb%2Fserver%2Fsrc%2Fsimcore_service_webserver%2Fapi_keys

No, I didn't figure out why some lines were reported as missing in the api_keys domain when those lines were in fact hit. But in this case of the catalog I was able to systematically reduce the missing lines by introduce tests that hit those part of the code. So that indicates I am using the tool correctly. Hence, I suspect the issue in the api_keys domain is that we have something misconfigured (our .coveragerc looks rather random TBH) or a bug in pytest-cov

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:catalog catalog service a:webserver issue related to the webserver service
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants