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

[DO NOT MERGE | PROOF OF CONCEPT] Convert the API to use FastAPI instead of APIFlask #204

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

chouinar
Copy link
Contributor

@chouinar chouinar commented Oct 5, 2023

This will not be merged, this is a proof-of-concept and may become its own separate repo from the flask repo (an alternative? a replacement? not sure yet).

I realize this is a massive set of changes, it may be easier to just check the code out yourself and look around instead.

Changes

Modified the API to use FastAPI rather than APIFlask

Updated Pydantic to 2.x which changes the naming on a few things like the settings classes.

Context for reviewers

This is a work-in-progress, there's a lot of TODOs that I still need to cleanup.

FastAPI docs: https://fastapi.tiangolo.com/

FastAPI is an alternative to Flask (and the many libraries built ontop of Flask like APIFlask or Connexion). It is built ontop of Starlette, and has a few noteworthy differences from Flask (from my experience of setting this up):

  • API schemas are defined as Pydantic models and you don't need to work with raw dictionaries
  • The middleware approach that FastAPI uses is incredibly flexible
  • Dependency Injection is very easy to do and requires minimal effort (eg. pulling in the DB session in an endpoint just requires proper typing).
  • All validation errors just rely on Pydantic, so any validation rules you might want to add as business rules (which would make sense to be done in Pydantic) could be added to those models potentially (A note in the TODO section below details testing this further)
  • Supports async endpoints and basically every example from their docs just assume you do that.

A few downsides I've found in my setup so far:

  • In our logging we automatically add the url_rule (eg. /users/{user_id}) to every request, but this value isn't known until much later in processing. I figured out a way to automate this using dependencies, but it does mean the first logs (eg. start request) don't have this value, but there's nothing that can be done about that.
  • Setting up a PATCH endpoint model is a pain. Let's assume you have a field like first_name that is not nullable in the DB. In a POST/PUT endpoint you'd just make the Pydantic request model have a type like str and it'll make sure it rejects anything that is null. But for a PATCH endpoint you're fine with it either being a str OR just not set at all, which you can only know if you give Pydantic a default. There's no partial model you can do like we were able to with Marshmallow. But, even with Marshmallow supporting that, I found PATCH endpoints to be difficult to work with
  • By default if you run into any validation issues on the response object, it just logs the validation issues, which includes the data itself. I worked around this by overriding the __str__ method on the validation error class, but that seems questionable (and was actually only added recently so was a deliberate decision).
  • There isn't a global way to make it so all responses get restructured like we previously had. It's possible with generics (Making everything return ApiResponse[YourResponseClassHere]), but I'm also not certain if we want that. Adding that would be feasible, and allow us to have a message/warning object on every response as well.
  • Our CLI commands had to be removed as they relied on using Flask to start them up. This will just require a new setup approach for CLI commands, which should be fairly minimal with how we've configured our logging/DB setup.

TODO:

  • Create a set of endpoints similar to ones we've used on other projects with much larger requests to verify that this will scale appropriately.
  • Add more validation rules for things like conditionally required fields (if you said X to question Y, field Z is required)
  • Look into response objects having a message/warnings like before
  • Need to verify if what I have configured breaks anything when running non-locally (not sure on how to go about that)
  • CLI commands - how could we do them in a clean way?

Testing

Rewrote any Flask-reliant test to work with FastAPI, thankfully there were minimal changes necessary (mostly switching the test client / response formats).

You can run the API locally by doing make init run-logs - might want to nuke whatever you have locally as a lot of dependencies were updated. The swagger docs can be accessed at http://localhost:8080/docs

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

Successfully merging this pull request may close these issues.

1 participant