The following setup doesn't work:

class ToDo(BaseModel):
    id: int
    item: str

todo_router = APIRouter()
todo_list = []

@todo_router.post("/todo", status_code=201)
async def add_todo(todo: ToDo) -> dict:
    for item in todo_list:
        if item.id == todo.id:
            raise HTTPException(
                status_code=409,
                detail="To-do with id already exists"
            )
        else:
            todo_list.append(todo)
            return {"message": "To-do added successfully"}

app = FastAPI()

app.include_router(todo_router)

It's got me stumped. I keep getting an Internal Server Error and the terminal error messages aren't very helpful. FYI I'm more used to Haskell-style Elm lang so the Python "magic" isn't so familiar to me anymore!

I know that Python returns work differently (the return value doesn't always need to be inside an else branch), and I think this works if I move it outside the else, but I don't understand why (I'm not confident of my code in Python!)

Perhaps someone can clarify this for me?

The following setup doesn't work:

class ToDo(BaseModel):
    id: int
    item: str

todo_router = APIRouter()
todo_list = []

@todo_router.post("/todo", status_code=201)
async def add_todo(todo: ToDo) -> dict:
    for item in todo_list:
        if item.id == todo.id:
            raise HTTPException(
                status_code=409,
                detail="To-do with id already exists"
            )
        else:
            todo_list.append(todo)
            return {"message": "To-do added successfully"}

app = FastAPI()

app.include_router(todo_router)

It's got me stumped. I keep getting an Internal Server Error and the terminal error messages aren't very helpful. FYI I'm more used to Haskell-style Elm lang so the Python "magic" isn't so familiar to me anymore!

I know that Python returns work differently (the return value doesn't always need to be inside an else branch), and I think this works if I move it outside the else, but I don't understand why (I'm not confident of my code in Python!)

Perhaps someone can clarify this for me?

• python
• fastapi
• pydantic

• 2 The else clause doesn't change anything - it's superfluous - either an exception happens inside your if check, or the code runs. The problem is that you call return inside your for loop, so the for loop never runs more than one iteration - it either errors with an exception or returns a value. Drop the else, and move append and return to the same indentation level as your for loop. – MatsLindh Commented Feb 2 at 22:41
• @MatsLindh I think I get it, but it's strange that this example code did run without errors. And I expect I can do a similar thing, to remove it's else and indenting at the same level as the for. So TL;DR let the loop run fully before .append() or raise? I think I'm used to having a branch for everything with case statements :) – Rob Commented Feb 3 at 22:30

2 Answers 2

You are getting Internal Server Error because of the response you are returning, which is nothing (None). At the beginning of your application startup, the todo_list will be empty. The for loop that you wrote, will not iterate at all.

You are doing a linear search, you should do this till the end of the list. If there is no match, then you should add, which you should have appended todo outside for loop:

from pydantic import BaseModel

from fastapi.responses import JSONResponse
from fastapi import FastAPI, APIRouter, status


class ToDo(BaseModel):
    id: int
    item: str


todo_router = APIRouter()
todo_list: list[ToDo] = []


@todo_router.post("/todo", status_code=status.HTTP_201_CREATED)
async def add_todo(todo: ToDo) -> JSONResponse:
    for existing_todo in todo_list:
        if existing_todo.id == todo.id:
            return JSONResponse(
                content={"detail": f"To-do with id={todo.id} already exists"},
                status_code=status.HTTP_409_CONFLICT,
            )

    todo_list.append(todo)
    return JSONResponse(
        content={"detail": "To-do added successfully"},
        status_code=status.HTTP_201_CREATED,
    )


app = FastAPI()
app.include_router(todo_router)

I also suggest returning JSONResponse over the Python dictionaries, because it's fast (FastAPI does not need to perform additional checks), and instead of using "magic" constants, use status instead where all status codes are defined.

You can also use dict(or something else) to speed up your searches for duplicates which is linear currently.

You iterating through 'todo_list' which is always empty:

todo_list = []

That means iteration won't be ever executed and router won't return anything. Just add return statement to the end of router function:

@todo_router.post("/todo", status_code=201)
async def add_todo(todo: ToDo) -> dict:
     for item in todo_list:
          if item.id == todo.id:
              raise HTTPException(
                  status_code=409,
                  detail="To-do with id already exists"
              )
         else:
             todo_list.append(todo)
             return {"message": "To-do added successfully"}
     return {"message": "Nothing to do!"}