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

✨ webserver-catalog rpc connection #6003

Merged
merged 69 commits into from
Jul 2, 2024

Conversation

pcrespov
Copy link
Member

@pcrespov pcrespov commented Jun 27, 2024

What do these changes do?

This PR exposes an rpc interface in the catalog and uses it in the webserver.
image

catalog

  • setup rabbitmq
  • ✨ new rcp server
    • implemented with fakes until coming PR

webserver's catalog plugin

  • _rpc client
  • _api module (service layer):
    - get catalog _rpc results and aggregates i/o
  • rest _handlers (controller layer)
    - implemented using _api

Related issue/s

How to test

  • tests rest entrypoints mocking catalog's _rpc client calls
cd services/web/server/
make install-dev
pytest -vv tests/unit/with_dbs/01/test_catalog_handlers__services.py:test_dev_get_and_patch_service

Dev-ops checklist

  • catalog service now interacts with rabbitmq. Added RABBITMQ_ related env-vars. No changes required in osparc-ops!

@pcrespov pcrespov self-assigned this Jun 27, 2024
@pcrespov pcrespov added a:webserver issue related to the webserver service a:catalog catalog service labels Jun 27, 2024
@pcrespov pcrespov added this to the South Island Iced Tea milestone Jun 27, 2024
Copy link

codecov bot commented Jun 27, 2024

Codecov Report

Attention: Patch coverage is 84.95298% with 48 lines in your changes missing coverage. Please review.

Project coverage is 87.8%. Comparing base (cafbf96) to head (7987fe3).
Report is 313 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #6003      +/-   ##
=========================================
+ Coverage    84.5%   87.8%    +3.2%     
=========================================
  Files          10    1425    +1415     
  Lines         214   58451   +58237     
  Branches       25    1396    +1371     
=========================================
+ Hits          181   51359   +51178     
- Misses         23    6798    +6775     
- Partials       10     294     +284     
Flag Coverage Δ
integrationtests 64.8% <30.4%> (?)
unittests 85.9% <84.9%> (+1.3%) ⬆️

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

Files Coverage Δ
...src/models_library/api_schemas_catalog/__init__.py 100.0% <100.0%> (ø)
...src/models_library/api_schemas_catalog/services.py 100.0% <100.0%> (ø)
...els_library/api_schemas_invitations/invitations.py 100.0% <100.0%> (ø)
...pi_schemas_resource_usage_tracker/pricing_plans.py 100.0% <100.0%> (ø)
...rc/models_library/api_schemas_webserver/catalog.py 100.0% <100.0%> (ø)
...ls_library/api_schemas_webserver/projects_nodes.py 96.9% <100.0%> (ø)
...ls_library/api_schemas_webserver/projects_ports.py 100.0% <100.0%> (ø)
...s/models-library/src/models_library/invitations.py 76.9% <100.0%> (ø)
...ages/models-library/src/models_library/payments.py 93.1% <100.0%> (ø)
...dels-library/src/models_library/projects_access.py 96.5% <100.0%> (ø)
... and 30 more

... and 1353 files with indirect coverage changes

@pcrespov pcrespov changed the title WIP: ✨ Is5964/webserver catalog cnx ✨ webserver-catalog rpc connection Jun 27, 2024
@pcrespov pcrespov marked this pull request as ready for review June 27, 2024 11: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.

Looking good, please adjust the code to our current convention for defining the RPC endpoints

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.

Looks good but please change the call back to is_old_service, maybe by adding a comment for it. thanks!

Copy link
Contributor

@bisgaard-itis bisgaard-itis left a comment

Choose a reason for hiding this comment

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

Cool, thanks a lot. I have a few questions (mostly for my understanding) below.

@pcrespov pcrespov force-pushed the is5964/webserver-catalog-cnx branch from 722846c to ea2c394 Compare June 28, 2024 09:05
@pcrespov pcrespov enabled auto-merge (squash) June 28, 2024 09:24
@pcrespov pcrespov force-pushed the is5964/webserver-catalog-cnx branch from 85c8415 to 91f835e Compare July 2, 2024 08:46
@pcrespov pcrespov force-pushed the is5964/webserver-catalog-cnx branch from 81a910a to 2f1b581 Compare July 2, 2024 15:13
@pcrespov pcrespov disabled auto-merge July 2, 2024 20:28
@pcrespov pcrespov merged commit c361c2f into ITISFoundation:master Jul 2, 2024
56 checks passed
@pcrespov pcrespov deleted the is5964/webserver-catalog-cnx branch July 3, 2024 11:37
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.

6 participants