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

♻️ directorv2 API schema models moved to models-library #4560

Merged

Conversation

pcrespov
Copy link
Member

@pcrespov pcrespov commented Jul 31, 2023

What do these changes do?

This PR moves the pydantic models in the director-v2 API (i.e. models/schemas) to models_library.api_schemas_directorv2. This way both server and client can share these models.
Changes only affect the director-v2 and models_library packages and the corresponding clients (e.g. web-server or api-server will be done in separate PRs).

  • ♻️ Moves directorv2's schemas to models-library.api_schemas_director_v2 and leaves in the service only domain models.

  • @sanderegg please follow up this PR by reviewing the openapi.json and the models in models_library.api_schemas_directorv2. After ♻️Maintenance: remove unused routes from director v2 #4462 the OAS changed but the openapi.json was not updated. Could you please have a look?.

Related issue/s

How to test

  • driving test: packages/models-library/tests/test__models_examples.py
  • current tests in directorv2

DevOps

None

@pcrespov pcrespov self-assigned this Jul 31, 2023
@pcrespov pcrespov added this to the Sundae milestone Jul 31, 2023
@pcrespov pcrespov added a:director-v2 issue related with the director-v2 service a:models-library labels Jul 31, 2023
@codecov
Copy link

codecov bot commented Jul 31, 2023

Codecov Report

Merging #4560 (2e53571) into master (72787f8) will decrease coverage by 1.1%.
The diff coverage is 95.3%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #4560     +/-   ##
========================================
- Coverage    85.6%   84.6%   -1.1%     
========================================
  Files        1007     842    -165     
  Lines       43186   38635   -4551     
  Branches     1001     495    -506     
========================================
- Hits        36996   32701   -4295     
+ Misses       5961    5827    -134     
+ Partials      229     107    -122     
Flag Coverage Δ
integrationtests 39.5% <ø> (-38.4%) ⬇️
unittests 84.4% <95.3%> (+0.2%) ⬆️

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

Files Changed Coverage Δ
...ages/models-library/src/models_library/services.py 91.3% <ø> (ø)
..._service_catalog/models/services_specifications.py 100.0% <ø> (ø)
...or-v2/src/simcore_service_director_v2/constants.py 100.0% <ø> (ø)
...ynamic_sidecar/scheduler/_core/_scheduler_utils.py 79.0% <69.2%> (-3.8%) ⬇️
...odels_library/api_schemas_directorv2/comp_tasks.py 87.5% <71.4%> (ø)
...service_director_v2/api/routes/dynamic_services.py 88.7% <75.0%> (-0.9%) ⬇️
.../models_library/api_schemas_directorv2/clusters.py 87.3% <100.0%> (ø)
...library/api_schemas_directorv2/dynamic_services.py 95.6% <100.0%> (ø)
...api_schemas_directorv2/dynamic_services_service.py 97.2% <100.0%> (ø)
...rc/models_library/api_schemas_directorv2/errors.py 100.0% <100.0%> (ø)
... and 45 more

... and 188 files with indirect coverage changes

@pcrespov pcrespov marked this pull request as ready for review July 31, 2023 09:14
@pcrespov pcrespov force-pushed the is4481/models_library_and_directorv2 branch from 67f587d to 2e53571 Compare July 31, 2023 09:15
@codeclimate
Copy link

codeclimate bot commented Jul 31, 2023

Code Climate has analyzed commit 2e53571 and detected 0 issues on this pull request.

View more on Code Climate.

@sonarcloud
Copy link

sonarcloud bot commented Jul 31, 2023

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

Copy link
Contributor

@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.

👍

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.

As a rule of thumb I would not move things away from director-v2. Especially if they are only used internally.

@pcrespov pcrespov merged commit 54970e3 into ITISFoundation:master Jul 31, 2023
50 checks passed
@pcrespov pcrespov deleted the is4481/models_library_and_directorv2 branch August 24, 2023 12:06
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Sep 22, 2023
50 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:director-v2 issue related with the director-v2 service a:models-library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants