full-stack-fastapi-template

full-stack-fastapi-template copied to clipboard

SQLAlchemy supports a built in get this is used in crud.delete but note crud.get, changing this will greatly enhance usability of the base.crud as it will work with tables where the primary key isn't named id as well.

Suggestion

Change this: https://github.com/tiangolo/full-stack-fastapi-postgresql/blob/490c554e23343eec0736b06e59b2108fdd057fdc/%7B%7Bcookiecutter.project_slug%7D%7D/backend/app/app/crud/base.py#L26-L27

To work like this: https://github.com/tiangolo/full-stack-fastapi-postgresql/blob/490c554e23343eec0736b06e59b2108fdd057fdc/%7B%7Bcookiecutter.project_slug%7D%7D/backend/app/app/crud/base.py#L62-L66

I've made this change a couple of weeks back in my project and it works great, no drawbacks but lots of benefits.

@tiangolo I've suggested a couple of improvements based on changes I've made in my own project. I could make a PR implementing all or some of these changes, if they are desirable in the upstream template. But I don't want to pollute your PR queue if they aren't desirable.

It's better to use Session.get because Query.get is deprecated since SQLAlchemy 1.4.

def get(self, db: Session, id: Any) -> Optional[ModelType]: 
     return db.get(self.model, id)

def remove(self, db: Session, *, id: Any) -> ModelType: 
     obj = db.get(self.model, id) 
     db.delete(obj) 
     db.commit() 
     return obj