full-stack-fastapi-template
full-stack-fastapi-template copied to clipboard
Default crud uses hardcoded id over general get from SQLAlchemy
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