Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Path parameter became required if it defined via Pydantic model #5019

Closed
9 tasks done
gtors opened this issue Jun 10, 2022 · 8 comments
Closed
9 tasks done

Path parameter became required if it defined via Pydantic model #5019

gtors opened this issue Jun 10, 2022 · 8 comments
Labels
question Question or problem question-migrate

Comments

@gtors
Copy link

gtors commented Jun 10, 2022

First Check

  • I added a very descriptive title to this issue.
  • I used the GitHub search to find a similar issue and didn't find it.
  • I searched the FastAPI documentation, with the integrated search.
  • I already searched in Google "How to X in FastAPI" and didn't find any information.
  • I already read and followed all the tutorial in the docs and didn't find an answer.
  • I already checked if it is not related to FastAPI but to Pydantic.
  • I already checked if it is not related to FastAPI but to Swagger UI.
  • I already checked if it is not related to FastAPI but to ReDoc.

Commit to Help

  • I commit to help with one of those options 👆

Example Code

from pydantic import BaseModel, Field
from fastapi import Path, FastAPI, Depends

app = FastAPI()

class Foo(BaseModel):
    bar: str = Field(Path('bar'))  # default value is defined

@app.get('/foo')  # always will return 422 because `bar` not represent in path
async def pathless_handler(params: Foo = Depends(Foo)):     
    pass

@app.get('/foo/{bar}')
async def path_aware_handler(params: Foo = Depends(Foo)):
    pass

Description

If the path parameters are defined via Pydantic model, then the path parameters become always required.

curl -v localhost:8000/foo
*   Trying 127.0.0.1:8000...
* Connected to localhost (127.0.0.1) port 8000 (#0)
> GET /foo HTTP/1.1
> Host: localhost:8000
> User-Agent: curl/7.83.1
> Accept: */*
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 422 Unprocessable Entity
< date: Fri, 10 Jun 2022 11:56:49 GMT
< server: uvicorn
< content-length: 87
< content-type: application/json
<
* Connection #0 to host localhost left intact
{"detail":[{"loc":["path","bar"],"msg":"field required","type":"value_error.missing"}]}% 

The root of evil is in this line (it makes a path parameter always required, even if a default value is specified 😢)

default=...,

Operating System

Linux

Operating System Details

No response

FastAPI Version

0.78.0

Python Version

3.10.4

Additional Context

No response

@gtors gtors added the question Question or problem label Jun 10, 2022
@gtors
Copy link
Author

gtors commented Jun 10, 2022

As ad-hoc solution:

from typing import Any
from fastapi.params import Path as OrigPath, Undefined

class Path(OrigPath):

    def __init__(self, default: Any = Undefined, *args, **kwargs):
        super().__init__(*args, **kwargs)

        if default is not Undefined:
            self.default = default

@NickVeld
Copy link

I also have found that the following blocks setting of default.

default=...,

If change it to default=default everything is fine.

I was wondering does it really have not been found before. But I have found this issue in the issue list.

Other related:

@NickVeld
Copy link

@gtors @triangolo
As I see it is not a technical problem to implement it. It is not implemented because of OpenAPI.

Now there is a following situation:

  • You are setting default
  • After some layers (even not in the top-level) it is discarded using =...
  • The developers does not understand anything and digging into the package code

My suggestion:

  1. Remove default at all (but I understand that this parameter can be handy when OpenAPI will be changed)
  2. Add a warning that signals that default will not be set.

I believe in both cases it is good to label the issue as a enchantment or a bug and not a question. Because nothing is asked but not intuitive behavior is shown.

I can create a pull request for both solutions.

@NickVeld
Copy link

NickVeld commented Sep 25, 2022

As for the ad-hoc solution, in order to be consistent the code must redefine Path from fastapi.param_functions (the usual import is from fastapi import Path) as well:

from typing import Any, Dict, Optional

from fastapi.params import Path as OriginalParamsPath, Undefined


class ParamsPath(OriginalParamsPath):
    """Original ``Path`` in ``fastapi.params`` implementation discards ``default``

    See https://github.com/tiangolo/fastapi/issues/5019
    """
    def __init__(self, default: Any = Undefined, *args, **kwargs):
        super().__init__(*args, **kwargs)

        if default is not Undefined:
            self.default = default


def Path(  # noqa: N802
    default: Any = Undefined,
    *,
    alias: Optional[str] = None,
    title: Optional[str] = None,
    description: Optional[str] = None,
    gt: Optional[float] = None,
    ge: Optional[float] = None,
    lt: Optional[float] = None,
    le: Optional[float] = None,
    min_length: Optional[int] = None,
    max_length: Optional[int] = None,
    regex: Optional[str] = None,
    example: Any = Undefined,
    examples: Optional[Dict[str, Any]] = None,
    deprecated: Optional[bool] = None,
    include_in_schema: bool = True,
    **extra: Any,
) -> Any:
    """Redefined version of Path from ``fastapi.param_functions``"""
    return ParamsPath(
        default=default,
        alias=alias,
        title=title,
        description=description,
        gt=gt,
        ge=ge,
        lt=lt,
        le=le,
        min_length=min_length,
        max_length=max_length,
        regex=regex,
        example=example,
        examples=examples,
        deprecated=deprecated,
        include_in_schema=include_in_schema,
        **extra,
    )

It is cleaner to place the whole snippet as-is into a separate module; and then import it from there.

@tiangolo
Copy link
Owner

Defining path parameters (or other parameters that don't come from the body) using Pydantic models is not really supported.

There will be a way to do that in a future release, but not yet. So, for now, your have to take that path parameter out of the Pydantic model.

@github-actions
Copy link
Contributor

Assuming the original need was handled, this will be automatically closed now. But feel free to add more comments or create new issues or PRs.

@NickVeld
Copy link

Defining path parameters (or other parameters that don't come from the body) using Pydantic models is not really supported.

There will be a way to do that in a future release, but not yet. So, for now, your have to take that path parameter out of the Pydantic model.

@tiangolo , it can be not implemented but the behavior stays confusing anyway. I proposed two solutions how to deal with it but you commented none of them.

@tiangolo
Copy link
Owner

Ah, yeah @NickVeld , I was focusing on the original post. So, the thing is, I'm not sure I understand what would be the intention of the original question, what would be expected in that case. I'm not sure what would mean to have a default value for a path parameter. I think I'm missing the explanation of what was expected, and what was @gtors intending to achieve.

Now, if you have a specific use case that needs something related to this, please create a new issue, following the template, and please make sure you add what are you intending to solve and what behavior you would have expected.

@tiangolo tiangolo reopened this Feb 27, 2023
@github-actions github-actions bot removed the answered label Feb 27, 2023
Repository owner locked and limited conversation to collaborators Feb 27, 2023
@tiangolo tiangolo converted this issue into discussion #6224 Feb 27, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
question Question or problem question-migrate
Projects
None yet
Development

No branches or pull requests

3 participants