-
-
Notifications
You must be signed in to change notification settings - Fork 8.4k
🐛 Reenable allow_arbitrary_types when only 1 argument is used on the API endpoint
#13694
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
🐛 Reenable allow_arbitrary_types when only 1 argument is used on the API endpoint
#13694
Conversation
…ent is used on the api endpoint
|
Also affected by issue:
Minimal Example: import fastapi
import uvicorn
from bson import ObjectId
from pydantic import BeforeValidator, PlainSerializer, WithJsonSchema, BaseModel, ConfigDict
from typing_extensions import Annotated, Optional
# Relevant reads
# https://docs.pydantic.dev/latest/concepts/json_schema/#implementing-__get_pydantic_json_schema__
# ObjectID for the backend parsing of the database models.
PydanticObjectId = Annotated[ObjectId,
BeforeValidator(ObjectId),
PlainSerializer(lambda x: str(x), when_used='json'),
WithJsonSchema({'type': 'string',
'examples': ["65e90eb121080ca48a56d7af", "65e90eb121080ca48a56d7b0"],
"description": "unique identifier for a document in the database. "
"24 hex characters."})]
class Request(BaseModel):
some_id: PydanticObjectId
model_config = ConfigDict(
arbitrary_types_allowed=True,
populate_by_name=True
)
app = fastapi.FastAPI(
title="Broken API",
)
@app.post("/broken")
async def broken(req: Request):
"""
This function should raise an Error 500 when called via api.
"""
print(f"Hello World")
@app.post("/works")
async def works(req: Optional[Request] = None):
"""
This function should work. For some reason adding Optional[] fixes the issue.
"""
print(f"Hello World")
if __name__ == "__main__":
uvicorn.run(app, host="
8000
0.0.0.0", port=8000)curiously, wrapping the argument of the endpoint in an Please merge and publish this fix asap |
allow_arbitrary_types when only 1 argument is used on the API endpoint
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Test fails before changes, this PR fixes it.
Changes look reasonable to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting! 🤔
Supporting arbitrary types sounds a bit strange to me, but with the example in the test, I think I understand. If you wanted to have a default "sentinel" value that is not one of the regular Python types. I still find it strange as there's also req.model_dump(exclude_unset=True), but maybe it makes sense for other additional use cases.
Also, the internal change doesn't seem to touch anything different than what would already happen in the expected cases. So, seems good, let's do it! ☕
This fixes a problem with the test case from #12504
The change means the if statement in
request_body_to_args()that was added in #12129 commit for Forms, doesn't run for non Form data, which was breaking this usage case.Further discussion on the problem is #13690 and #12503
The test case was pulled from the PR by @RB387 above.