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

Inconsistent add_api_route types #10236

Open
9 tasks done
Kludex opened this issue Sep 12, 2023 Discussed in #10235 · 7 comments · May be fixed by #10240
Open
9 tasks done

Inconsistent add_api_route types #10236

Kludex opened this issue Sep 12, 2023 Discussed in #10235 · 7 comments · May be fixed by #10240
Labels
bug Something isn't working confirmed easy close good first issue Good for newcomers

Comments

@Kludex
Copy link
Sponsor Collaborator

Kludex commented Sep 12, 2023

Discussed in #10235

Originally posted by sidekick-eimantas September 12, 2023

First Check

  • I added a very descriptive title here.
  • I used the GitHub search to find a similar question 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

### Direct mounting of routes
import pathlib

from fastapi import FastAPI, APIRouter
import uvicorn
import pydantic

class HelloRequest(pydantic.BaseModel):
    id: str


class HelloResponse(pydantic.BaseModel):
    hello: str


class Hello:
    def __init__(self, name: str) -> None:
        self.name = name

    async def handle(self, request: HelloRequest) -> HelloResponse:
        return HelloResponse(hello=self.name)

app = FastAPI()
hello_handler = Hello(name="test")
app.add_api_route("/hello", hello_handler.handle, methods=["POST"])






### Mounting of paths via Router
import pathlib

from fastapi import FastAPI, APIRouter
import uvicorn
import pydantic

class HelloRequest(pydantic.BaseModel):
    id: str


class HelloResponse(pydantic.BaseModel):
    hello: str


class Hello:
    def __init__(self, name: str) -> None:
        self.name = name

    async def handle(self, request: HelloRequest) -> HelloResponse:
        return HelloResponse(hello=self.name)


class RootRouter:
    def __init__(self, name: str):
        self.router = APIRouter()
        hello_handler = Hello(name=name)
        self.router.add_api_route("/hello", hello_handler.handle, methods=["POST"])


app = FastAPI()
root_router = RootRouter(name="test")
app.include_router(root_router.router)

Description

First example produces a mypy error:

error: Argument 2 to "add_api_route" of "FastAPI" has incompatible type "Callable[[HelloRequest], Coroutine[Any, Any, HelloResponse]]"; expected "Callable[..., Coroutine[Any, Any, Response]]"  [arg-type]

Second example does not.

The types of endpoint parameter in FastAPI.add_api_route and APIRouter.add_api_route are inconsistent.

Operating System

macOS

Operating System Details

No response

FastAPI Version

0.103.1

Pydantic Version

2.3.0

Python Version

3.10.7

Additional Context

No response

@Kludex Kludex added the question Question or problem label Sep 12, 2023
@Kludex
Copy link
Sponsor Collaborator Author

Kludex commented Sep 12, 2023

This is a valid issue. The type hint of both endpoint parameters should match.

Should be easy to fix - I don't know which one @tiangolo wants.

@Kludex Kludex added confirmed bug Something isn't working good first issue Good for newcomers easy close and removed question Question or problem labels Sep 12, 2023
@annis-souames
Copy link

Hey @Kludex

I want to help on this issue if no one is interested, I have used FastAPI before in some hobby projects but this is my first OSS contribution!

@ordinary-jamie
Copy link

ordinary-jamie commented Sep 12, 2023

I'll take this if no one else is!

I don't know which one tiangolo wants.

I would think Callable[..., Any] is preferred:

  • FastAPI.add_api_route makes a direct call to APIRouter.add_api_route anyways
  • fastapi.dependencies.utils.get_typed_return_annotation has signature def get_typed_return_annotation(call: Callable[..., Any]) -> Any:
  • A lot of other functions with similar parameters use the same typing (e.g. FastAPI.add_api_websocket_route)
  • The api router instantiates route_class: Type[APIRoute] of which fastapi.routing.APIRoute subclasses starlette.routing.Route which also types endpoint with Callable[..., typing.Any]

Happy to work with either

@Alshymy22
Copy link

Direct mounting of routes

import pathlib

from fastapi import FastAPI, APIRouter
import uvicorn
import pydantic

class HelloRequest(pydantic.BaseModel):
id: str

class HelloResponse(pydantic.BaseModel):
hello: str

class Hello:
def init(self, name: str) -> None:
self.name = name

async def handle(self, request: HelloRequest) -> HelloResponse:
    return HelloResponse(hello=self.name)

app = FastAPI()
hello_handler = Hello(name="test")
app.add_api_route("/hello", hello_handler.handle, methods=["POST"])

Mounting of paths via Router

import pathlib

from fastapi import FastAPI, APIRouter
import uvicorn
import pydantic

class HelloRequest(pydantic.BaseModel):
id: str

class HelloResponse(pydantic.BaseModel):
hello: str

class Hello:
def init(self, name: str) -> None:
self.name = name

async def handle(self, request: HelloRequest) -> HelloResponse:
    return HelloResponse(hello=self.name)

class RootRouter:
def init(self, name: str):
self.router = APIRouter()
hello_handler = Hello(name=name)
self.router.add_api_route("/hello", hello_handler.handle, methods=["POST"])

app = FastAPI()
root_router = RootRouter(name="test")
app.include_router(root_router.router

@eydenbarboza
Copy link

hi @Kludex Is this still a valid issue? I want to help on this issue if no one is interested

@devkan
Copy link

devkan commented Mar 14, 2024

Hi.. is closed? I like FastAPI and want to contribute..

@eydenbarboza
Copy link

Hi, this issue is closed ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working confirmed easy close good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants