Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

@kaiix
Copy link

@kaiix kaiix commented Jan 12, 2022

@kaiix kaiix force-pushed the response-model-revalidate branch from 5489df0 to 28c24dc Compare January 13, 2022 03:00
fastapi/routing.py Outdated Show resolved Hide resolved
fastapi/routing.py Show resolved Hide resolved
@codecov

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@github-actions
Copy link
Contributor

📝 Docs preview for commit a9b3e8c at: https://639cd42098600317dc000812--fastapi.netlify.app

@dolfinus
Copy link
Contributor

dolfinus commented Mar 5, 2023

@tiangolo any chance this PR will be reviewed?

@Tishka17
Copy link

Tishka17 commented Mar 5, 2023

Rebuilding a response model is a real problem and cannot actually be done correctly because of aliases and other settings (I guess I've already left a comment about this). In normal code your create it by your own from the other sources that could not be converted automatically so it is actually not needed.

Can we expect this to be reviewed and fixed?

@nkhitrov
Copy link

@tiangolo hi! Can you answer on this comment?

#4416 (comment)

@levsh
Copy link

levsh commented Jun 6, 2023

Hi!
I have this problem too (
This can greatly degrade performance for the complex models with validators or endpoints that return a large number of items.
In example below get_items endpoint already returns a list of items so it is no need to rebuild response again.
And finally we have response in ~2 second instead of ~1 second.

from typing import List
from uuid import UUID, uuid4

from fastapi import FastAPI
from pydantic import BaseModel, validator

app = FastAPI()

class Data(BaseModel):
    i: int

    @validator("i")
    def validate_i(cls, v):
        sleep(0.1)
        return v

class Item(BaseModel):
    id: UUID
    data: Data

@app.get(
    "/items",
    response_model=List[Item],
)
def get_items():
    return [Item(id=uuid4(), data=Data(i=i)) for i in range(10)]

Thank you!

@Tishka17
Copy link

Tishka17 commented Jul 6, 2023

@tiangolo are there any plans on merging or reimplementing this?

@Danipulok
Copy link

@tiangolo is there any way this PR could be merged?

@tiangolo tiangolo added feature New feature or request p4 and removed investigate labels Jan 15, 2024
@patrick91 patrick91 changed the title Don't revalidate the response content if it is of the same type as response_model ✨ Don't revalidate the response content if it is of the same type as response_model Aug 9, 2024
Copy link
Contributor

@patrick91 patrick91 left a comment

Choose a reason for hiding this comment

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

Looks good to me, I rebased it and update the tests for Pydantic 2 :)

tests/test_serialize_response_model.py Outdated Show resolved Hide resolved
@kaiix kaiix force-pushed the response-model-revalidate branch from d2946c3 to b582069 Compare February 28, 2025 16:04
@kaiix
Copy link
Author

kaiix commented Feb 28, 2025

Updated codes and tests.

When users are using Pydantic v2 (fortunately, most applications should be using v2 by now), this issue doesn’t need much attention.

In v2, field.validate will call type_adapter.validator. If you check its schema, you’ll see that the validator will not revalidate instances of response_model (see reference: revalidate_instances).

[This is part of the validator schema for the AutoIncrement object in the test case]

 SchemaValidator(title="AutoIncrement", validator=Model(
    ModelValidator {
        revalidate: Never,
        validator: ModelFields(
            ModelFieldsValidator {
                fields: [
                    Field {
                        name: "count",
                        lookup_key: Simple {
                            key: "count",
...

Although there are some additional function calls, it won’t trigger the validator in a way that would modify the original response_content value in certain cases.

Therefore, this fix is only relevant for applications that are still using Pydantic v1.

@kaiix kaiix force-pushed the response-model-revalidate branch from b582069 to b5fcef9 Compare March 3, 2025 13:14
@kaiix kaiix force-pushed the response-model-revalidate branch from b5fcef9 to ec08e23 Compare March 3, 2025 13:31
def test_response_model_should_not_revalidate_response_content_if_they_had_same_type():
response = client.post("/increment")
response.raise_for_status()
assert response.json() == {"count": 1}
Copy link
Member

@svlandeg svlandeg Sep 12, 2025

Choose a reason for hiding this comment

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

Just to confirm: on master and with Pydantic 1.10.22, this fails with

AssertionError: assert {'count': 2} == {'count': 1}

@github-actions github-actions bot added the conflicts Automatically generated when a PR has a merge conflict label Sep 29, 2025
@github-actions

This comment was marked as resolved.

@github-actions github-actions bot removed the conflicts Automatically generated when a PR has a merge conflict label Sep 29, 2025
@svlandeg svlandeg marked this pull request as draft September 29, 2025 08:36
@svlandeg
Copy link
Member

svlandeg commented Sep 29, 2025

The changes from #14099 are interfering with this PR... 🕵️‍♀️

[UPDATE]: solved ✔️

@svlandeg svlandeg marked this pull request as ready for review September 29, 2025 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature or request p4

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Morty Proxy This is a proxified and sanitized view of the page, visit original site.