Skip to content

Fixes on catalog/services section of the web-API #2151

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

Merged
merged 28 commits into from
Feb 24, 2021

Conversation

pcrespov
Copy link
Member

@pcrespov pcrespov commented Feb 15, 2021

What do these changes do?

Requests in the catalog/services section of the web-API are validated in the webserver and forwarded to the catalog. The responses of the catalog are then again processed in the webserver (adds convenience values for the units) and retransmitted backt o the front-end

  • @odeimaiz: list_services return the same inputs as get_service_input via the web-API
    • webAPI operations: list_services, update_service and get_service now have explicit handlers
    • json serialization done with orjson
  • activated /catalog/services ... web API (no need for WEBSERVER_DEV_FEATURES_ENABLED=1)
  • prunes invalid field defaultValue from output ports

catalog service

  • upgrades dependencies
  • added model examples and extended test coverage
  • reduces size of responses bodies by trimming all null-ed attributes (search for RESPONSE_MODEL_POLICY)

Related issue/s

How to test

make build
make up-prod

Checklist

  • Moves some functionality from api-server+web-server to the catalog service: partially moved models, rest -> Next PR
  • Alleviates database connection load -> Next PR
  • Simplify catalog API models -> Next PR
  • Improve error handling: partially, next -> Next PR
  • prune & upgrade all requirements
  • create service-fastapi-library -> Next PR
  • Did you change any service's API? Then make sure to bundle document and upgrade version (make openapi-specs, git commit ... and then make version-*)
  • Unit tests for the changes exist
  • Runs in the swarm
  • Documentation reflects the changes
  • New module? Add your github username to .github/CODEOWNERS

@pcrespov pcrespov self-assigned this Feb 15, 2021
@pcrespov pcrespov added a:api framework api, data schemas, a:catalog catalog service dependencies labels Feb 15, 2021
@pcrespov pcrespov added this to the The White Rabbit milestone Feb 15, 2021
@codecov
Copy link

codecov bot commented Feb 15, 2021

Codecov Report

Merging #2151 (91dce39) into master (d6750a3) will increase coverage by 3.9%.
The diff coverage is 35.1%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #2151     +/-   ##
========================================
+ Coverage    73.1%   77.0%   +3.9%     
========================================
  Files         464     249    -215     
  Lines       17709   10326   -7383     
  Branches     1741    1019    -722     
========================================
- Hits        12946    7957   -4989     
+ Misses       4313    2053   -2260     
+ Partials      450     316    -134     
Flag Coverage Δ
integrationtests 65.5% <35.1%> (-0.3%) ⬇️
unittests 69.2% <35.1%> (+2.6%) ⬆️

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

Impacted Files Coverage Δ
...er/src/simcore_service_webserver/catalog_client.py 52.3% <16.6%> (-3.8%) ⬇️
.../simcore_service_webserver/catalog_api_handlers.py 35.6% <31.4%> (+0.2%) ⬆️
.../src/simcore_service_webserver/diagnostics_core.py 50.8% <33.3%> (-38.7%) ⬇️
...rc/simcore_service_webserver/catalog_api_models.py 55.5% <40.9%> (-20.7%) ⬇️
...eb/server/src/simcore_service_webserver/catalog.py 95.6% <100.0%> (-0.3%) ⬇️
.../simcore-sdk/src/simcore_sdk/node_data/__init__.py 0.0% <0.0%> (-100.0%) ⬇️
...core-sdk/src/simcore_sdk/node_data/data_manager.py 0.0% <0.0%> (-95.3%) ⬇️
...erver/src/simcore_service_webserver/rest_models.py 0.0% <0.0%> (-91.2%) ⬇️
...src/simcore_service_sidecar/celery_configurator.py 0.0% <0.0%> (-91.2%) ⬇️
...src/simcore_service_webserver/activity/handlers.py 27.1% <0.0%> (-62.9%) ⬇️
... and 260 more

@pcrespov pcrespov changed the title Enhance/catalog api WIP: Enhance/catalog api Feb 15, 2021
@pcrespov pcrespov marked this pull request as draft February 15, 2021 22:56
@pcrespov pcrespov force-pushed the enh/catalog-api branch 3 times, most recently from 9f0d4fa to 064b086 Compare February 22, 2021 21:38
@pcrespov pcrespov changed the title WIP: Enhance/catalog api WIP: Fixes on catalog/services section of the web-API Feb 23, 2021
@pcrespov pcrespov changed the title WIP: Fixes on catalog/services section of the web-API WIP: Fixes on catalog/services section of the web-API Feb 23, 2021
@pcrespov pcrespov changed the title WIP: Fixes on catalog/services section of the web-API Fixes on catalog/services section of the web-API Feb 23, 2021
@pcrespov pcrespov marked this pull request as ready for review February 23, 2021 10: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.

👍

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.

We partially discussed together and I thought we agreed that exlude_none is not good.

if you have a field that can be nullable but has a default of 2 for example. say I set None, then on export this field will disappear and when it comes back it will set as 2 by default.

@pcrespov pcrespov requested a review from sanderegg February 23, 2021 20:16
@pcrespov pcrespov merged commit 18f6042 into ITISFoundation:master Feb 24, 2021
@pcrespov pcrespov deleted the enh/catalog-api branch February 24, 2021 06:06
@sanderegg sanderegg mentioned this pull request Feb 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:api framework api, data schemas, a:catalog catalog service
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants