Skip to content

🎨 Exposes get_service_ports to rpc interface of the catalog simcore-service #7558

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 19 commits into from
Apr 23, 2025
Merged

🎨 Exposes get_service_ports to rpc interface of the catalog simcore-service #7558

merged 19 commits into from
Apr 23, 2025

Conversation

@pcrespov
Copy link
Member

@pcrespov pcrespov commented Apr 22, 2025 •
edited
Loading

What do these changes do?

Exposes get_service_ports to rpc interface of the catalog simcore-service

These are all the parts modified in this PR:
image

  1. rpc-server in catalog simcore-service
    • builds on top of service-layer
  2. rpc-client in servicelib
  3. mocks helpers in pytest_simcore
  4. new service-layer at api-server simcore-service
    • wraps rpc-client
    • NOTE: this could have been even implemented using the rest-client at the same time as the rpc-client
  5. old service-layer wrapping rest-client at api-server simcore-service

NOTE: that the rest API in catalog simcore-service is still in use but some of the entrypoints could be slowly being replaced by the rpc API implementations.

Related issue/s

How to test

cd services/catalog
make install-dev
pytest -vv tests/unit/test_service_manifest.py
pytest -vv tests/unit/test_api_rpc.py

cd services/api-server
make install-dev
pytest -vv tests/unit/test_services_catalog.py

Dev-ops

None

@pcrespov pcrespov added the a:catalog catalog service label Apr 22, 2025
@pcrespov pcrespov self-assigned this Apr 22, 2025
@codecov
Copy link

codecov bot commented Apr 22, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 89.39394% with 7 lines in your changes missing coverage. Please review.

Project coverage is 87.65%. Comparing base (9d0e894) to head (34825f6).
Report is 1 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #7558      +/-   ##
==========================================
+ Coverage   87.61%   87.65%   +0.04%     
==========================================
  Files        1758     1752       -6     
  Lines       68131    67987     -144     
  Branches     1124     1124              
==========================================
- Hits        59690    59594      -96     
+ Misses       8132     8084      -48     
  Partials      309      309
Flag Coverage Δ
integrationtests 65.13% <ø> (-0.03%) ⬇️
unittests 86.84% <89.39%> (+0.04%) ⬆️
Components Coverage Δ
api ∅ <ø> (∅)
pkg_aws_library 93.91% <ø> (ø)
pkg_dask_task_models_library 97.09% <ø> (ø)
pkg_models_library 92.70% <100.00%> (+<0.01%) ⬆️
pkg_notifications_library 85.26% <ø> (ø)
pkg_postgres_database 88.18% <ø> (ø)
pkg_service_integration 69.98% <ø> (ø)
pkg_service_library 72.71% <0.00%> (-0.07%) ⬇️
pkg_settings_library 90.90% <ø> (ø)
pkg_simcore_sdk 85.40% <ø> (ø)
agent 96.46% <ø> (ø)
api_server 91.24% <100.00%> (+0.01%) ⬆️
autoscaling 96.08% <ø> (ø)
catalog 92.65% <100.00%> (+0.18%) ⬆️
clusters_keeper 99.25% <ø> (ø)
dask_sidecar 91.29% <ø> (ø)
datcore_adapter 98.12% <ø> (ø)
director 76.78% <ø> (ø)
director_v2 91.37% <ø> (ø)
dynamic_scheduler 97.40% <ø> (ø)
dynamic_sidecar 90.11% <ø> (ø)
efs_guardian 89.79% <ø> (ø)
invitations 93.28% <ø> (ø)
payments 92.66% <ø> (ø)
resource_usage_tracker 89.12% <ø> (-0.11%) ⬇️
storage 87.58% <ø> (-0.08%) ⬇️
webclient ∅ <ø> (∅)
webserver 86.10% <ø> (+0.06%) ⬆️

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 9d0e894...34825f6. 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.

@pcrespov pcrespov changed the title WIP: Is7532/catalog rpc filter get service ports 🎨 Exposes get_service_ports to rpc interface of the catalog simcore-service Apr 22, 2025
@pcrespov pcrespov added this to the Pauwel Kwak milestone Apr 22, 2025
@pcrespov pcrespov force-pushed the is7532/catalog-rpc-filter-get_service_ports branch from 5685941 to b199f28 Compare April 22, 2025 15:51
@pcrespov pcrespov marked this pull request as ready for review April 22, 2025 15:51
@pcrespov pcrespov enabled auto-merge (squash) April 22, 2025 16:46
Copilot AI reviewed Apr 22, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR exposes the get_service_ports functionality to the RPC interface of the catalog simcore-service. Key changes include:

  • Replacing direct calls to legacy service functions with calls to catalog_rpc.get_service_ports in both tests and service layers.
  • Introducing new functions in the service manifest and catalog_services modules to retrieve ports.
  • Updating API endpoints, RPC client interfaces, and related tests/mocks to support the new service ports retrieval.

Reviewed Changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated no comments.

Show a summary per file
File Description
services/catalog/tests/unit/with_dbs/test_api_rpc.py Updated tests to call catalog_rpc.* functions and added tests for get_service_ports.
services/catalog/tests/unit/test_service_manifest.py Adjusted fixtures and tests to validate the new service ports retrieval.
services/catalog/src/simcore_service_catalog/service/manifest.py Added get_service_ports function to extract input/output ports from a service.
services/catalog/src/simcore_service_catalog/service/catalog_services.py Updated service queries to use new get_user_services_ports function wrapping the manifest.
services/catalog/src/simcore_service_catalog/api/rpc/_services.py Exposed get_service_ports via the RPC interface.
services/api-server/* Updated tests and endpoint implementations to incorporate the new get_service_ports functionality.
packages/service-library and pytest-simcore Enhanced mocks and RPC interface definitions to include get_service_ports support.
packages/models-library Renamed port conversion method from from_service_io to from_domain_model for consistency.
Files not reviewed (1)
  • services/catalog/openapi.json: Language not supported

@pcrespov pcrespov added the a:apiserver api-server service label Apr 22, 2025
giancarloromeo
giancarloromeo approved these changes Apr 22, 2025
View reviewed changes
Copy link
Contributor

@giancarloromeo giancarloromeo left a comment

Choose a reason for hiding this comment

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

Good.

bisgaard-itis
bisgaard-itis approved these changes Apr 23, 2025
View reviewed changes
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

sanderegg
sanderegg approved these changes Apr 23, 2025
View reviewed changes
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.

Thanks! Please check my comments.

@pcrespov pcrespov force-pushed the is7532/catalog-rpc-filter-get_service_ports branch from 1df3f65 to 492c9f5 Compare April 23, 2025 08:11
@pcrespov pcrespov added the 🤖-automerge marks PR as ready to be merged for Mergify label Apr 23, 2025
@pcrespov
Copy link
Member Author

@mergify queue

@mergify
Copy link
Contributor

mergify bot commented Apr 23, 2025 •
edited
Loading

queue

🟠 Waiting for conditions to match

  • -closed [📌 queue requirement]
  • -conflict [📌 queue requirement]
  • -draft [📌 queue requirement]
  • any of: [📌 queue -> configuration change requirements]
    • -mergify-configuration-changed
    • check-success = Configuration changed
  • any of: [🔀 queue conditions]
    • all of: [📌 queue conditions of queue default]
      • #approved-reviews-by >= 2 [🛡 GitHub branch protection]
      • #approved-reviews-by>=2
      • #changes-requested-reviews-by = 0 [🛡 GitHub branch protection]
      • #changes-requested-reviews-by=0
      • #review-threads-unresolved = 0 [🛡 GitHub branch protection]
      • #review-threads-unresolved=0
      • -conflict
      • -draft
      • base=master
      • branch-protection-review-decision = APPROVED [🛡 GitHub branch protection]
      • label!=🤖-do-not-merge
      • label=🤖-automerge
      • any of: [🛡 GitHub branch protection]
        • check-skipped = deploy to dockerhub
        • check-neutral = deploy to dockerhub
        • check-success = deploy to dockerhub
      • any of: [🛡 GitHub branch protection]
        • check-success = system-tests
        • check-neutral = system-tests
        • check-skipped = system-tests
      • any of: [🛡 GitHub branch protection]
        • check-success = unit-tests
        • check-neutral = unit-tests
        • check-skipped = unit-tests
      • any of: [🛡 GitHub branch protection]
        • check-success = check OAS' are up to date
        • check-neutral = check OAS' are up to date
        • check-skipped = check OAS' are up to date
      • any of: [🛡 GitHub branch protection]
        • check-success = integration-tests
        • check-neutral = integration-tests
        • check-skipped = integration-tests
      • any of: [🛡 GitHub branch protection]
        • check-success = [build] docker images (excluding frontend) (3.11, ubuntu-24.04)
        • check-neutral = [build] docker images (excluding frontend) (3.11, ubuntu-24.04)
        • check-skipped = [build] docker images (excluding frontend) (3.11, ubuntu-24.04)

@pcrespov pcrespov force-pushed the is7532/catalog-rpc-filter-get_service_ports branch from fcc5c78 to 934b1ef Compare April 23, 2025 09:26
@sonarqubecloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@pcrespov pcrespov merged commit 69ccf1f into ITISFoundation:master Apr 23, 2025
94 checks passed
@pcrespov pcrespov deleted the is7532/catalog-rpc-filter-get_service_ports branch April 23, 2025 11:44
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request May 8, 2025
Closed
34 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@giancarloromeo giancarloromeo giancarloromeo approved these changes

@sanderegg sanderegg sanderegg approved these changes

@bisgaard-itis bisgaard-itis bisgaard-itis approved these changes

@matusdrobuliak66 matusdrobuliak66 Awaiting requested review from matusdrobuliak66 matusdrobuliak66 is a code owner

Assignees

@pcrespov pcrespov

Labels

🤖-automerge marks PR as ready to be merged for Mergify a:apiserver api-server service a:catalog catalog service

Projects

None yet

Milestone

Pauwel Kwak

Development

Successfully merging this pull request may close these issues.

4 participants

@pcrespov @giancarloromeo @sanderegg @bisgaard-itis