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

[REQ] [python-fastapi] Better support for spec-first development #13863

Open
sherifnada opened this issue Oct 30, 2022 · 4 comments
Open

[REQ] [python-fastapi] Better support for spec-first development #13863

sherifnada opened this issue Oct 30, 2022 · 4 comments

Comments

@sherifnada
Copy link

sherifnada commented Oct 30, 2022

Is your feature request related to a problem? Please describe.

The current python-fastapi generator seems to be geared towards a one-time generation of the python server rather than a continuous spec-first approach. For example:

Let's say I have the following OpenAPI spec

openapi: 3.0.0
info:
  description: |
    My API

  version: "1.0.0"
  title: my API
  contact:
    email: [email protected]
  license:
    name: MIT
    url: "https://opensource.org/licenses/MIT"

paths:
  /hello_world:
    get:
      description: "Sample endpoint"
      responses:
        "200":
          description: Successful operation
          content:
            application/json:
              schema:
                type: object
                properties:
                  hello:
                    type: string

When I run openapi-generator -i <path_to_spec> -g python-fastapi, a file src/openapi/apis/default_api.py (among other things) is autogenerated containing the following code:

router = APIRouter()

@router.get(
    "/hello_world",
    responses={
        200: {"model": InlineResponse200, "description": "Successful operation"},
    },
    tags=["default"],
)
async def hello_world_get(
) -> InlineResponse200:
    """Sample endpoint"""
    ...

The problem with this code gen pattern is that it doesn't enable a spec-first workflow because it doesn't separate API contract declaration from its implementation. That is, if I implement the generated hello_world_get method, then regenerate the file from the OpenAPI spec, my implementation gets overwritten.

If this was say, Java, the endpoints would be encapsulated under an interface like:

// DefaultAPI.java
// This interface is autogenerated 
interface DefaultAPI() {
  InlineResponse200 helloWorld();
}

Then in a separate file I'd implement the API

// DefaultApiImplementation.java
class DefaultApiImplementation implements DefaultAPI {
  InlineResponse200 helloWorld() {
    // ... 
  }
}

If a new method is added to the spec or an existing method is updated, I would run the generator and it would update DefaultAPI.java with any new parameters, return types, API endpoints, etc... And because the implementation (which I create by hand) implements that interface, the build will fail if one of the signatures doesn't correctly match or if an endpoint is not implemented.

Describe the solution you'd like

I'd love to start a conversation about whether the generator intends to evolve in this direction or if I'm just misunderstanding its usage pattern. Ideally, the code generated above enables two development experience invariants:

  1. allow separating the implementation of api endpoints from their declaration
  2. detect if an API method in the spec was not implemented by the developer or implemented with incorrect signatures and raise an exception during CI or tests

I have some outlines of a solution and would love some feedback. Basically, we'd change the structure of the generator to have the following output components:

  1. for each API, there is an ABC declaring the methods a developer needs to implement. Each such method corresponds to one API endpoint.
  2. For each API, the user implements a class which extends the ABC from step 1 in a separate file
  3. The user manually has to write the main.py file which initializes the FastAPI instance which actually launches the server, using the implementation they created from step 2 to initialize any routers.

I've included some pseudocode below that demoes what this would look like on the OpenAPI spec above:

default_api.py (this file is autogenerated):

class APIInterface(ABC): 
  # autogenned, contains pythonic signature of api
  @abstractmethod
  async def hello_world():
     """ hello world """


def initialize_router(api: APIInterface) -> APIRouter:
    router = APIRouter()
    # Directly add the method as a route implementation 
    # instead of adding via decoration (which is the current approach)
    router.get(
        "/hello_world",
        responses={
            200: {"model": InlineResponse200, "description": "Successful operation"},
        },
        tags=["default"],
    )
    
    # for each endpoint, do the same thing...
    # ...
    
    # at the end just return the router
    return router

api_implementation.py (this file is written and maintained by the user):

from default_api import APIInterface

# handwritten implementation of the API methods
class APIImplementation(APIInterface):
  async def hello_world():
    # code code code 

main.py (this file is written and maintained by the user, although we can probably automate some parts of it. Not essential that we do that for the sake of this discussion):

# The user must manually write this class
from default_api import initialize_router
from api_implementation import APIImplementation

app = FastAPI(
    title="my API",
    description=" API ",
    version="1.0.0",
)

app.include_router(initialize_router(APIImplementation()))

Would love some feedback on this approach and whether there is any appetite for adopting it.

@sherifnada
Copy link
Author

Tagging @spacether as a member of the Python technical committee and @krjakbrjak as the package creator. Any feedback on the problem above and the suggested solution?

@thatguysimon
Copy link

@sherifnada is this concept implemented in any of the other server generators?

@ff137
Copy link

ff137 commented May 27, 2023

For those curious to follow up, see: fastapi/fastapi#6169 (comment)

@wing328
Copy link
Member

wing328 commented May 29, 2023

recently we merged #14470 into the master to better support spec-first development.

please pull the latest master to give it a try.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants