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

feat: typed request / response schema #1752

Merged
merged 23 commits into from
Dec 12, 2023
Merged

feat: typed request / response schema #1752

merged 23 commits into from
Dec 12, 2023

Conversation

kyujin-cho
Copy link
Member

@kyujin-cho kyujin-cho commented Dec 2, 2023

This PR introduces Pydantic based request and response schema validation, along with automatic OpenAPI spec generation of those who've opted in for Pydantic based handler instead of traditional trafaret way.

What's changed

support for Pydantic validator on both request and response body

Now you can pass a Pydantic model, which inherits BaseModel, as a request body validator.

# trafaret way
@auth_required
@server_status_required(READ_ALLOWED)
@check_api_params(
    t.Dict(
        {
            t.Key("traffic_ratio"): t.Float[0:],
        }
    ),
)
async def update_route(request: web.Request, params: Any) -> web.Response:
...

# pydantic way
class UpdateRouteRequestModel(BaseModel):
    traffic_ratio: NonNegativeFloat


@auth_required
@server_status_required(READ_ALLOWED)
@check_api_params(UpdateRouteRequestModel)
async def update_route(request: web.Request, params: UpdateRouteRequestModel) -> web.Response:
...

There are two major advantages of choosing Pydantic in place of of Trafaret: much faster, and native type inference support for params parameter (which will act as a parsed request body). Please note that, after transiting the validator to Pydantic model, you can no longer access values described under request body via string index (params["abc"] way).
Same way also applies for the response schema:

# web.json_response() way
async def scale(request: web.Request, params: Any) -> web.Response:
...
        return web.json_response(
            {"current_route_count": len(endpoint.routings), "target_count": params["to"]}
        )

# trafaret way
async def scale(request: web.Request, params: ScaleRequestModel) -> ScaleResponseModel:
...
        return ScaleResponseModel(
            current_route_count=len(endpoint.routings), target_count=params.to
        )

Note the difference of function return type annotation between two handlers. Explicitly defining the response type will help Backend.AI's automated OpenAPI spec generator understand the response schema and render it as OpenAPI spec.

On-the-fly REST API reference viewer

Two new GET resources (/spec, /spec/spec.json) are newly introduced on this patch.

  • GET /spec: returns HTML page containing a REST API reference documentation for Backend.AI. Please keep in mind that this page won't work under the air-gapped environment.
    image
  • GET /spec/spec.json: returns OpenAPI-3.0.0 compliant JSON schema of whole Backend.AI REST API
    The entire /spec resource is disabled by default, so in order to activate these system administrator has to set value of config/api/allow-openapi-schema-introspection etcd key to yes.

Checklist: (if applicable)

  • Milestone metadata specifying the target backport version
  • Documentation

@github-actions github-actions bot added comp:manager Related to Manager component size:XL 500~ LoC labels Dec 2, 2023
@kyujin-cho kyujin-cho modified the milestones: 23.09, 24.03 Dec 2, 2023
@kyujin-cho kyujin-cho requested a review from achimnol December 2, 2023 08:13
@kyujin-cho kyujin-cho added impact:invisible This change is invisible to users (internal changes). skip:changelog Make the action workflow to skip towncrier check labels Dec 2, 2023
Comment on lines 178 to 181
if isinstance(checker, t.Trafaret):
checked_params = checker.check(stripped_params)
else:
checked_params = checker.model_validate(stripped_params)
Copy link
Member

Choose a reason for hiding this comment

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

For extra type safety, let's use the pattern matching.

Suggested change
if isinstance(checker, t.Trafaret):
checked_params = checker.check(stripped_params)
else:
checked_params = checker.model_validate(stripped_params)
match checker:
case t.Trafaret():
checked_params = checker.check(stripped_params)
case BaseModel():
checked_params = checker.model_validate(stripped_params)

@kyujin-cho kyujin-cho requested a review from achimnol December 4, 2023 11:21
Copy link
Member

@achimnol achimnol left a comment

Choose a reason for hiding this comment

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

I've added a commit to keep consistency of the decorated request handler's typing. The request handler should use Callable[[web.Request, ...], web.StreamResponse] always, so that middleware could be composed in any order and additional strong-typed decorators do not get confused.

Currently I couldn't find a good way to apply typing.overload to embrace both the trafaret version and the pydantic version of check_api_params(), and thus split out check_api_params_v2() for new codes.

Please work on the remaining migration that I've left as TODOs, and also ensure the OpenAPI doc generator's implementation to work with check_api_params_v2().

@kyujin-cho kyujin-cho requested a review from achimnol December 5, 2023 07:16
@kyujin-cho kyujin-cho added this pull request to the merge queue Dec 12, 2023
Merged via the queue into main with commit e9480e4 Dec 12, 2023
25 checks passed
@kyujin-cho kyujin-cho deleted the feature/pydantic branch December 12, 2023 03:15
@kyujin-cho kyujin-cho mentioned this pull request Jan 8, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:manager Related to Manager component impact:invisible This change is invisible to users (internal changes). size:XL 500~ LoC skip:changelog Make the action workflow to skip towncrier check
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants