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

♻️ project CRUD routes: updates OAS and model schemas (part 1) #4064

Merged
merged 95 commits into from
Apr 21, 2023

Conversation

pcrespov
Copy link
Member

@pcrespov pcrespov commented Apr 4, 2023

What do these changes do?

This PR reviews the specs and implementation of CRUD operations on the project resource. The OAS now exposes a PATCH as well and all the schemas have been reviewed:

image

This PR is very verbose and therefore, for the sake of clarity, is the first of many parts.

Highlights

  • ✨ Reviews schemas used i/o operations with project and creates multiple pydantic models for projects in projects/_rest_schemas.py
    • see test_projects__rest_schemas.py where new models are tested against data captured from real fe-be communications.
    • OAS auto-generated with api/specs/webserver/scripts/openapi_projects_crud.py
  • ✨ New PATCH /project/* interface (Implementation will follow in next PR)
  • ♻️ refactors project_crud_handlers
    • separated implementation under projects/_utils*.py
    • POST /projects validate now request body with new models
  • 🗑️ api/schemas files
    • validation is directly done using pydantic models
  • 📝 docs raised expections
  • Upgrade of mypy in ⬆️Maintenance: update mypy to 1.2.0 #4108 resurfaced new issues
    • 🔨 fixes sqlalchemy plugin config
    • ♻️ fixes mypy issues in packages/postgres-database
    • ♻️ fixes mypy issues in catalog

Related issue/s

How to test

cd services/web/server
make install-dev
pytest -vv tests/unit/isolated/test_project*.py

@pcrespov pcrespov self-assigned this Apr 4, 2023
@pcrespov pcrespov changed the title Is4059/project patch part1 WIP: Is4059/project patch part1 Apr 4, 2023
@codecov
Copy link

codecov bot commented Apr 4, 2023

Codecov Report

Merging #4064 (6b105be) into master (1d49346) will decrease coverage by 0.1%.
The diff coverage is 95.3%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #4064     +/-   ##
========================================
- Coverage    85.4%   85.4%   -0.1%     
========================================
  Files         952     954      +2     
  Lines       41350   41294     -56     
  Branches      962     946     -16     
========================================
- Hits        35350   35283     -67     
- Misses       5782    5794     +12     
+ Partials      218     217      -1     
Flag Coverage Δ
integrationtests 67.2% <80.1%> (+0.6%) ⬆️
unittests 82.3% <95.3%> (-0.1%) ⬇️

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

Impacted Files Coverage Δ
...e/src/simcore_postgres_database/models/projects.py 100.0% <ø> (ø)
...library/src/servicelib/aiohttp/application_keys.py 100.0% <ø> (ø)
...rary/src/servicelib/aiohttp/requests_validation.py 90.2% <ø> (ø)
...e_api_server/utils/solver_job_models_converters.py 88.0% <ø> (ø)
...2/src/simcore_service_director_v2/core/settings.py 98.4% <ø> (-0.1%) ⬇️
...server/src/simcore_service_webserver/_constants.py 100.0% <ø> (ø)
...r/src/simcore_service_webserver/projects/plugin.py 100.0% <ø> (ø)
...r/src/simcore_service_webserver/socketio/plugin.py 100.0% <ø> (ø)
...rvice_webserver/projects/projects_handlers_crud.py 87.7% <71.7%> (-5.5%) ⬇️
...rary/src/models_library/utils/common_validators.py 77.7% <77.7%> (ø)
... and 20 more

... and 5 files with indirect coverage changes

@ITISFoundation ITISFoundation deleted a comment from codeclimate bot Apr 4, 2023
@ITISFoundation ITISFoundation deleted a comment from codeclimate bot Apr 4, 2023
@ITISFoundation ITISFoundation deleted a comment from codeclimate bot Apr 4, 2023
@ITISFoundation ITISFoundation deleted a comment from codeclimate bot Apr 4, 2023
@pcrespov pcrespov force-pushed the is4059/project-patch-part1 branch from 82871a6 to 8b6e9e1 Compare April 4, 2023 19:27
@pcrespov pcrespov added the a:webserver issue related to the webserver service label Apr 4, 2023
@pcrespov pcrespov added this to the Jelly Beans milestone Apr 5, 2023
@pcrespov pcrespov force-pushed the is4059/project-patch-part1 branch 3 times, most recently from 11f1632 to f4c146e Compare April 12, 2023 16:26
Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 5118 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 14393 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 14407 lines exceeds the maximum allowed for the inline comments feature.

@pcrespov pcrespov force-pushed the is4059/project-patch-part1 branch from 61a4db3 to 92c2a1b Compare April 12, 2023 20:56
Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 14850 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 14849 lines exceeds the maximum allowed for the inline comments feature.

@pcrespov pcrespov changed the title WIP: Is4059/project patch part1 ♻️ project CRUD routes: updates OAS and model schemas (part 1) Apr 12, 2023
@pcrespov pcrespov marked this pull request as ready for review April 12, 2023 21:16
Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 14882 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 14886 lines exceeds the maximum allowed for the inline comments feature.

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.

14k files... you're doing a competition? ;)
looking forward to seeing my PRs in conflicts...

@pcrespov pcrespov force-pushed the is4059/project-patch-part1 branch from 2707818 to 8a637dd Compare April 20, 2023 20:35
Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 24666 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 24677 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 24677 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 24677 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 24738 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 24757 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 24850 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 24880 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 24880 lines exceeds the maximum allowed for the inline comments feature.

@codeclimate
Copy link

codeclimate bot commented Apr 21, 2023

Code Climate has analyzed commit 6b105be and detected 0 issues on this pull request.

View more on Code Climate.

@sonarcloud
Copy link

sonarcloud bot commented Apr 21, 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

@pcrespov pcrespov merged commit 12763d6 into ITISFoundation:master Apr 21, 2023
@pcrespov pcrespov deleted the is4059/project-patch-part1 branch April 21, 2023 14:13
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request May 30, 2023
24 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:webserver issue related to the webserver service
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants