Skip to content

pass-through for Schema-based results #1497

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

andreasnuesslein
Copy link

When the response-parameter is filled and the result is already a Schema we don't need to verify it again.

Brief reasoning for the situation:
Wagtail has a [new guide how to integrate Ninja](based on https://docs.wagtail.org/en/v7.0/advanced_topics/api/django-ninja.html).
In it they use this

@api.get("/pages/{page_id}/", response=BlogPageSchema | HomePageSchema)
def get_page(request: "HttpRequest", page_id: int):
    return get_object_or_404(Page, id=page_id).specific

This works but has its issues since basically the returned Page type (the django Model) is tried to be fitted against all different response-Schemata. Obviously that's a performance issue but it brings other problems too, which I dont want to go too deep into.

So my solution would be:

@api.get("/pages/{page_id}/", response=BlogPageSchema | HomePageSchema)
def get_page(request: "HttpRequest", page_id: int):
    page = get_object_or_404(Page, id=page_id).specific
    if isinstance(page,BlogPage):
        return BlogPageSchema.from_orm(page,context={"request":request})
    return HomePageSchema.from_orm(page,context={"request":request})

this does currently not work, because the result is being re-parsed, because I have response set in the decorator. Obviously I could remove that but then I'd lose the types.

This PR allows Schemas to be directly returned, assuming that the validation already has happened.

Thanks so much

When the response-parameter is filled and the result is already a Schema we don't need to verify it again.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant