4
\$\begingroup\$

I am new to the FastApi world. Can somebody review my below Fast API design? It is working but doesn't follow FastApi conventions. I find it much cleaner and follows layered architecture taking inspiration from Java Spring-boot landscape. What are pros and cons of this desing.

routes -> services -> crud -> models and using schemas.

Can like Springboot I can have something like @Autowired

plant_model.py

from sqlalchemy import Column, Integer, String, Text
from app.database.postgres import Base

class Plant(Base):
    __tablename__ = "plant"
    id = Column(Integer, primary_key=True, index=True)
    name = Column(String, index=True)
    type = Column(String)
    description = Column(Text)

plant_schemas.py

from pydantic import BaseModel

class PlantBase(BaseModel):
    name: str
    type: str

class PlantCreate(PlantBase):
    pass

class PlantInDB(PlantBase):
    id: int

class PlantPublic(PlantInDB):
    pass

plant_crud.py

from typing import List
from fastapi import APIRouter, Depends, HTTPException
from sqlalchemy.orm import Session
from app.database.postgres import SessionLocal
from .plant_schemas import PlantCreate, PlantInDB
from .plant_models import Plant


class PlantCrud:

    def __init__(self, db: Session = SessionLocal()) -> None:
        self.db = db

    def create_plant(self, plantCreate: PlantCreate):
        db_plant = Plant(plantCreate.model_dump())
        self.db.add(db_plant)
        self.db.commit()    
        return db_plant

    def get_plants(self, skip: int = 0, limit: int = 100):
        return self.db.query(Plant).offset(skip).limit(limit).all()

plant_service.py

from fastapi import Depends
from app.api.plant.plant_crud import PlantCrud
from app.api.plant.plant_schemas import PlantCreate

class PlantService:

    def __init__(self, plantCrud: PlantCrud = PlantCrud()) -> None:
        self.plantCrud = plantCrud

    def create_plant(self, plantCreate: PlantCreate):
        return self.plantCrud.create_plant(plantCreate)

    def get_plants(self, skip: int = 0, limit: int = 100):
        return self.plantCrud.get_plants(skip, limit)

plant_routes.py

from app.api.plant.plant_models import Plant
from app.api.plant.plant_schemas import PlantCreate, PlantInDB
from app.api.plant.plant_service import PlantService
from fastapi import Depends, APIRouter, HTTPException
from fastapi import APIRouter, Depends

router = APIRouter()

class PlantRoutes:

    def __init__(self, plantService: PlantService = PlantService()) -> None:
        self.plant_service = plantService

    def get_plants(self):
        return self.plant_service.get_plants()
    
    def create_plant(self, plantCreate: PlantCreate):
        return self.plant_service.create_plant(plantCreate)


plantRoutes = PlantRoutes()
router.add_api_route("/plants", plantRoutes.get_plants, methods=["GET"], response_model=list[PlantInDB])
router.add_api_route("/plants", plantRoutes.create_plant, methods=["POST"], response_model=PlantInDB, status_code=201)

main.py

# app/__init__.py
from fastapi import FastAPI
import os
import sys
from fastapi.middleware.cors import CORSMiddleware

current_script_dir = os.path.dirname(os.path.realpath(__file__))

# Add the parent directory (project directory) to sys.path
sys.path.append(os.path.dirname(current_script_dir))


my_app = FastAPI()
origins = [
   "*"
]
my_app.add_middleware(
   CORSMiddleware,
   allow_origins=origins,
   allow_credentials=True,
   allow_methods=["*"],
   allow_headers=["*"],
)

from app.api.plant.plant_routes import router as plant_router
my_app.include_router(plant_router, prefix="/plant", tags=["plant"])

if __name__ == "__main__":
    import uvicorn
    from main import my_app
    uvicorn.run("main:my_app", host="127.0.0.1", reload=True)
```
\$\endgroup\$
1
  • \$\begingroup\$ Since you are using SQLAlchemy and Pydantic as part of your study you should checkout SQLModel \$\endgroup\$
    – Bob
    Mar 27 at 23:31

1 Answer 1

3
\$\begingroup\$

missing docstring

These are obvious and lovely:

class PlantBase(BaseModel):
    name: str
    type: str

class PlantInDB(PlantBase):
    id: int

These are obscure:

class PlantCreate(PlantBase):
    pass

class PlantPublic(PlantInDB):
    pass

Clearly you intend they should be different in some way. But you neglected to tell us about the difference. Use a docstring for that.

The PlantService class appears to be all boilerplate. Please give it a docstring which explains why we need it, and how it differs from PlantCrud.

appropriate name

I read these two verbs and couldn't believe my eyes.

    def create_plant( ... ):
        ...
        self.db.add(db_plant)
        self.db.commit()    

Then I came back to this:

    def __init__(self, db: Session = SessionLocal()) -> None:
        self.db = db

OIC, it is not a DB at all, it is a session. So call it a session, or perhaps self.sess.

And it's still not clear to me what distinct role PlantCreate plays.

Pep-8 asks that you spell the parameter plant_create instead of plantCreate. Similarly for plant_crud.

mutable default parameter

Evaluating an expression just once, at import time, rather than N times for N invocations, can be surprising behavior for the unsuspecting maintenance engineer.

    def __init__(self, db: Session = SessionLocal()) -> None:

Prefer this common idiom.

    def __init__(self, db: Session = None)) -> None:
        self.db = db or SessionLocal()

fix PYTHONPATH

# Add the parent directory (project directory) to sys.path
sys.path.append(os.path.dirname(current_script_dir))

No, please don't do that. Arrange for sys.path to be configured properly from the get go, during initial interpreter startup. Set the env var prior to invocation, perhaps using a make target or perhaps using a shebang like this:

#! /usr/bin/env PYTHONPATH=../..:/some/where python

Also, please keep all your main.py imports together at top of file. The __main__ guard is nice enough, but it's not like anyone will be doing import main, there's simply no app code to test. A unit test might go faster by avoiding an expensive import uvicorn, but for this code there won't be any such tests.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.