dmontagu / fastapi-utils

Reusable utilities for FastAPI
MIT License
1.9k stars 165 forks source link

CRUD tools #15

Open nyejon opened 4 years ago

nyejon commented 4 years ago

Hi @dmontagu

Nice work here.

I have developed some CRUD tools, using the FastAPI postgres template "CRUD" base that works together with your CBV. I have also added sorting and filtering using sqlalchemy_filters that can be defined for each CBV.

This should make it much easier to build a basic CRUD app for fast-api.

The filter /sort fields can work through joins too.

This allows for any endpoint to take defined parameters like: /api/v1/modelname/?limit=100&field_1=eq:something&model2_field_1=eq:somethingelse&sort_by=field1:desc,model2field1:asc

Basically this includes a CRUD base for the database with each model inheriting from this. A base CBV that will be used for the standard CRUD stuff.

I think this could work well being in this separate project instead of in the cookiecutter template.

Could you see this fit into the scope of this project?

Thanks, Jonathan

dmontagu commented 4 years ago

Hi @nyejon,

I do think this could be a good fit. The most important thing for me would be that it feels generic/reusable as opposed to targeting a more domain-specific workflow. Based on your description above, it sounds like what you have in mind meets the bar.

I'm hesitant to add another dependency (sqlalchemy_filters), but would be willing to consider it if it substantially simplifies the implementation. Ideally though, it would be possible to make use of the functionality without requiring the use of sqlalchemy_filters for people who don't want to add another transitive dependency and don't require its usage. (If you've already essentially hard-coded sqlalchemy_filters as a dependency I'd be happy to take a look at the implementation and see if there is a clean way to extract it, and to do some of that work myself if I see it as a barrier to merging.)

Would you like to make a PR? (If so, feel free -- I'll happily review it.)

dmontagu commented 4 years ago

Also, something to think about -- does this have any relationship to generic CBVs as described in #2 and #3? If practical usage and/or implementation would benefit from the existence of generic CBVs that might be a good reason to push to get that implemented.

nyejon commented 4 years ago

Yes it does fit into the generic CBV as that is ultimately what I am aiming for.

I also had trouble with making a base view reusable like in #2 and ended up making a work around which I am not particularly happy with. For each CBV I used the Depends to get the filter parameters that I defined in a separate method. It would be nice if filter_fields could be defined as part of the class.

I like django-rest-framework and how that was implemented, and use it as a reference. I agree with you on the external dependencies that a CBV shouldn't require:

  1. SQL Alchemy
  2. sqlalchemy-filters

What we could define though is a way to extract filter and sort parameters, validate them with pydantic and then make them available to a user with recommendations about how to use them?

dmontagu commented 4 years ago

For what it's worth, I'm fine with providing specializations that require additional dependencies. I would just prefer to offer a simpler version if it can provide other benefits.

I also think your proposal about extracting parameters makes sense, but I would have a strong preference that the approach be type-safe and autocompletion-friendly, rather than relying on dynamic behaviors (e.g., something like specifying a classvar tuple/list of strings to look for for filters). In my experience, dynamic-heavy approaches often lead to large reductions in the amount of code necessary, but I find it difficult to maintain code using such features, and also difficult to discover how those features work by just reviewing code. I'm not sure whether the approach you have in mind would match this design goal or not, but I'd be happy to help get it there if you open a PR.

To the extent that we want to support more dynamic filter parameters, I suspect something like GraphQL might be better-suited (though I'm not very familiar with how filters work in a GraphQL context). In general, I would prefer to avoid things that translate poorly to the auto-generated OpenAPI schema. (But GraphQL may provide a more flexible alternative, which is why I bring it up.)