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

🎨 web-server api: ordering parameters and simplified openapi specs for complex query parameters #6737

Merged
merged 42 commits into from
Nov 18, 2024

Conversation

pcrespov
Copy link
Member

@pcrespov pcrespov commented Nov 15, 2024

What do these changes do?

ReDoc Swagger UI
Unifies and simplifies ordering queries. For instance, list folders is displayed as
image
from this coded openapi specification

@router.get(
    "/folders",
    response_model=Envelope[list[FolderGet]],
)
async def list_folders(
    _q: Annotated[as_query(FolderListQueryParams), Depends()],
):
    ...    

♻️ Unified ordering query parameters

  • SEE packages/models-library/src/models_library/rest_ordering.py
  • Introduced a factory function to streamline the handling of ordering parameters, ensuring flexibility for customization per case.
  • Refactored existing resources (folders, projects, ...) to adopt the unified approach for defining ordering parameters.
  • Established a naming convention for models: $(ResourceName)$(OperationName)OrderQueryParams (e.g., FoldersListOrderQueryParams).

♻️ Simplified handling of complex query parameters (i.e. JSON-encoded)

  • Unified the logic with a single class for both OpenAPI specification generation and request validation.
    • Includes the helper function as_query for ease of use
  • Improved implementation in api/specs/web-server to reduce complexity and potential errors.. E.g. openapi specs are simply defined using one model per "group" of parameters
  • Note that it uses dependency injection shortcuts to expand all fields in the model as (path _p/ query _q) parameters.

Related issue/s

How to test

Driving test

cd pakcages/models-library
make install-dev
pytest -vv tests/test_rest_ordering

Dev-ops

None

@pcrespov pcrespov self-assigned this Nov 15, 2024
Copy link

codecov bot commented Nov 15, 2024

Codecov Report

Attention: Patch coverage is 94.68085% with 10 lines in your changes missing coverage. Please review.

Project coverage is 85.70%. Comparing base (5f70a0d) to head (d85312a).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6737      +/-   ##
==========================================
- Coverage   88.33%   85.70%   -2.63%     
==========================================
  Files        1440     1573     +133     
  Lines       58398    63309    +4911     
  Branches     1405     2125     +720     
==========================================
+ Hits        51583    54257    +2674     
- Misses       6615     8739    +2124     
- Partials      200      313     +113     
Flag Coverage Δ
integrationtests ?
unittests 85.70% <94.68%> (-0.80%) ⬇️
Components Coverage Δ
api 79.60% <ø> (ø)
pkg_aws_library 93.38% <ø> (ø)
pkg_dask_task_models_library 96.87% <ø> (ø)
pkg_models_library 91.93% <85.71%> (-0.09%) ⬇️
pkg_notifications_library 83.79% <ø> (ø)
pkg_postgres_database 87.33% <ø> (ø)
pkg_service_integration 71.44% <ø> (ø)
pkg_service_library 76.54% <100.00%> (∅)
pkg_settings_library 91.42% <ø> (ø)
pkg_simcore_sdk 65.69% <ø> (-19.58%) ⬇️
agent 96.98% <ø> (ø)
api_server 89.88% <ø> (ø)
autoscaling 95.22% <ø> (ø)
catalog 89.42% <ø> (ø)
clusters_keeper 98.72% <ø> (ø)
dask_sidecar 91.32% <ø> (ø)
datcore_adapter 94.05% <ø> (ø)
director 58.38% <ø> (-0.05%) ⬇️
director_v2 84.32% <ø> (-6.51%) ⬇️
dynamic_scheduler 96.59% <ø> (ø)
dynamic_sidecar 88.83% <ø> (-0.95%) ⬇️
efs_guardian 90.01% <ø> (ø)
invitations 93.51% <ø> (ø)
osparc_gateway_server 39.84% <ø> (-45.58%) ⬇️
payments 92.87% <ø> (ø)
resource_usage_tracker 90.65% <ø> (-0.15%) ⬇️
storage 89.76% <ø> (ø)
webclient ∅ <ø> (∅)
webserver 87.23% <98.46%> (-0.36%) ⬇️

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 5f70a0d...d85312a. Read the comment docs.

---- 🚨 Try these New Features:

@pcrespov pcrespov added the a:webserver issue related to the webserver service label Nov 15, 2024
@pcrespov pcrespov added this to the Event Horizon milestone Nov 15, 2024
@pcrespov pcrespov changed the title WIP: Is468/rest ordering 🎨 web-server api: ordering parameters and simplified generation of openapi specs for complex query parameters Nov 15, 2024
@pcrespov pcrespov changed the title 🎨 web-server api: ordering parameters and simplified generation of openapi specs for complex query parameters 🎨 web-server api: ordering parameters and simplified openapi specs for complex query parameters Nov 15, 2024
@pcrespov pcrespov force-pushed the is468/rest-ordering branch from 82d2ea8 to 7827e0d Compare November 15, 2024 20:39
@pcrespov pcrespov marked this pull request as ready for review November 15, 2024 20:43
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.

Very nice refactoring.
This is the inverse of parse_as_body/header/param/query methods. nice!
Just a few comments and questions.

api/specs/web-server/_common.py Show resolved Hide resolved
api/specs/web-server/_projects_crud.py Outdated Show resolved Hide resolved
api/specs/web-server/_resource_usage.py Outdated Show resolved Hide resolved
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.

Thanks so much! It’s really nice. I’ve left a few comments.

@pcrespov pcrespov force-pushed the is468/rest-ordering branch from 2b9b63b to 911f54e Compare November 18, 2024 12:02
@pcrespov pcrespov enabled auto-merge (squash) November 18, 2024 12:04
@pcrespov pcrespov force-pushed the is468/rest-ordering branch from 911f54e to 357ba30 Compare November 18, 2024 15:26
@pcrespov pcrespov force-pushed the is468/rest-ordering branch from 357ba30 to d85312a Compare November 18, 2024 16:16
@pcrespov pcrespov merged commit 625d2cb into ITISFoundation:master Nov 18, 2024
82 of 89 checks passed
@pcrespov pcrespov deleted the is468/rest-ordering branch November 19, 2024 18:31
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