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

♻️ Is3515/ deprecates servicelib.extract_and_validate #3691

Merged

Conversation

pcrespov
Copy link
Member

@pcrespov pcrespov commented Dec 15, 2022

What do these changes do?

The main goal is to remove the dependency from openapi-core introduced in servicelib.aiohttp. As a first PR, this will replace servicelib.rest_utils.extract_and_validate by pydantic models that validate requests from serviceslib.aiohttp.requests_validation

  • ♻️ Removes dependencies on servicelib.rest_utils.extract_and_validate from all services except webserver's storageplugin (will do in a separate PR)
  • ♻️ Refactors webserver's storage plugin:
    • Cleanup opeapi-storage.yml and creates autogenerator api/specs/webserver/scripts/openapi_storage.py
    • Defines storage_schemas pydantic models
    • Next PR will remove extract_and_validate from this module
  • 🔨 new helper pytest_simcore.pydantic_models.iter_model_examples_in_module

Related issue/s

How to test

Checklist

  • make openapi.json (no change)
  • Unit tests for the changes exist
  • Runs in the swarm
  • Documentation reflects the changes

@codecov
Copy link

codecov bot commented Dec 15, 2022

Codecov Report

Merging #3691 (83fcb4d) into master (b84c3e5) will increase coverage by 1.2%.
The diff coverage is 99.3%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #3691     +/-   ##
========================================
+ Coverage    83.2%   84.4%   +1.2%     
========================================
  Files         848     884     +36     
  Lines       35603   37551   +1948     
  Branches      787     785      -2     
========================================
+ Hits        29634   31718   +2084     
+ Misses       5759    5623    -136     
  Partials      210     210             
Flag Coverage Δ
integrationtests 67.5% <20.6%> (+6.4%) ⬆️
unittests 81.7% <99.3%> (+0.2%) ⬆️

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

Impacted Files Coverage Δ
...s/service-library/src/servicelib/rest_constants.py 0.0% <ø> (ø)
...ver/src/simcore_service_webserver/rest_handlers.py 100.0% <ø> (ø)
...rvice-library/src/servicelib/aiohttp/rest_utils.py 75.0% <75.0%> (+8.3%) ⬆️
...age/src/simcore_service_storage/handlers_health.py 100.0% <100.0%> (ø)
.../src/simcore_service_webserver/catalog_handlers.py 43.3% <100.0%> (ø)
...src/simcore_service_webserver/clusters/handlers.py 100.0% <100.0%> (+2.9%) ⬆️
...mcore_service_webserver/login/api_keys_handlers.py 95.6% <100.0%> (ø)
...imcore_service_webserver/meta_modeling_handlers.py 84.2% <100.0%> (ø)
...rvice_webserver/projects/projects_handlers_crud.py 93.1% <100.0%> (ø)
...r/src/simcore_service_webserver/storage_schemas.py 100.0% <100.0%> (ø)
... and 67 more

@pcrespov pcrespov self-assigned this Dec 15, 2022
@pcrespov pcrespov force-pushed the is3515/rm-extract_and_validate branch from 78a79e9 to e1e41da Compare December 15, 2022 19:27
@pcrespov pcrespov added the t:maintenance Some planned maintenance work label Dec 15, 2022
@pcrespov pcrespov added this to the Zefram Cochrane milestone Dec 15, 2022
@pcrespov pcrespov marked this pull request as ready for review December 15, 2022 19:36
@pcrespov pcrespov changed the title WIP: Is3515/rm extract and validate Is3515/rm extract and validate Dec 15, 2022
@pcrespov pcrespov changed the title Is3515/rm extract and validate ♻️ Is3515/ deprecates servicelib.extract_and_validate Dec 15, 2022
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 nice.
Question: will this already speedup the boot time?

@pcrespov pcrespov force-pushed the is3515/rm-extract_and_validate branch from 57c0d05 to 51e0b31 Compare December 16, 2022 08:15
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.

Great! very much looking forward to removing these old things!
Can you just double check my 2 comments before merging. thanks!

@pcrespov pcrespov force-pushed the is3515/rm-extract_and_validate branch from 51e0b31 to 0743d7a Compare December 16, 2022 11:06
@sonarcloud
Copy link

sonarcloud bot commented Dec 16, 2022

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

@pcrespov pcrespov merged commit e158d98 into ITISFoundation:master Dec 16, 2022
@pcrespov pcrespov deleted the is3515/rm-extract_and_validate branch December 16, 2022 14:26
@pcrespov pcrespov restored the is3515/rm-extract_and_validate branch December 16, 2022 14:26
@pcrespov pcrespov deleted the is3515/rm-extract_and_validate branch December 16, 2022 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t:maintenance Some planned maintenance work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants