Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .flake8
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[flake8]
inline-quotes = "
ignore = E203, E266, W503, ANN002, ANN003, ANN101, ANN102, ANN401, N807, N818, VNE001, F401
max-line-length = 119
max-complexity = 18
select = B,C,E,F,W,T4,B9,ANN,Q0,N8,VNE
exclude = .venv, __pycache__, migrations, alembic
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ __pycache__/

# C extensions
*.so

# Distribution / packaging
.Python
build/
Expand Down
82 changes: 82 additions & 0 deletions crud.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
from fastapi import HTTPException
from sqlalchemy import select
from sqlalchemy.orm import Session

import schemas
from db import models


def get_all_books(
db: Session,
author_id: int | None = None,
skip: int | None = None,
limit: int | None = None,
) -> list[models.Book]:

query = db.query(models.Book)
if author_id is None:
query = query.where(models.Book.author_id == author_id)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic for filtering by author_id is inverted. This if condition only applies the filter when author_id is None, but the requirement is to filter when an author_id is provided. You should check if author_id is not None.

query = query.limit(limit).offset(skip)

return list(db.scalars(query))
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a mix of SQLAlchemy query styles here. db.query() returns a Query object (legacy 1.x style), but db.scalars() expects a select() construct (modern 2.0 style). To execute the Query object and get a list of results, you should use .all() instead, like so: query.all().



def get_author_by_id(
db: Session,
author_id: int
) -> models.Author | None:
author = db.execute(
select(models.Author).where(models.Author.id == author_id)
).scalar_one_or_none()
if not author:
raise HTTPException(status_code=404, detail="Author not found")
return author


def create_book(
db: Session,
book: schemas.BookCreateSchema
) -> models.Book:
author = db.execute(
select(models.Author).where(models.Author.id == book.author_id)
).scalar_one_or_none()

if not author:
raise HTTPException(status_code=404, detail="Author not found")
db_book = models.Book(
title=book.title,
summary=book.summary,
publication_date=book.publication_date,
author_id=book.author_id
)
db.add(db_book)
db.commit()
db.refresh(db_book)

return db_book


def create_author(
db: Session,
author: schemas.AuthorCreateSchema
) -> models.Book:
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function's return type hint is models.Book, but the implementation creates and returns an models.Author object on line 72. The type hint should be updated to models.Author to match the actual return value.


db_author = models.Author(
name=author.name,
bio=author.bio,
)
db.add(db_author)
db.commit()
db.refresh(db_author)

return db_author


def get_authors_list(
db: Session,
skip: int | None = None,
limit: int | None = None,
) -> list[models.Author]:
query = db.query(models.Author).offset(skip).limit(limit)

return list(db.scalars(query))
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to the get_all_books function, this line mixes SQLAlchemy API styles. The query object was created with db.query(), so you should call query.all() to retrieve the list of authors instead of using db.scalars().

Empty file added db/__init__.py
Empty file.
12 changes: 12 additions & 0 deletions db/database.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
from sqlalchemy import create_engine
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.orm import sessionmaker

SQLALCHEMY_DATABASE_URL = "sqlite:///./db.db"

engine = create_engine(
SQLALCHEMY_DATABASE_URL, connect_args={"check_same_thread": False}
)
SessionLocal = sessionmaker(autocommit=False, autoflush=False, bind=engine)

Base = declarative_base()
31 changes: 31 additions & 0 deletions db/models.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import datetime
from sqlalchemy import String, Date, ForeignKey
from sqlalchemy.orm import mapped_column, Mapped, relationship

from db.database import Base


class Author(Base):
__tablename__ = "authors"

id: Mapped[int] = mapped_column(primary_key=True, index=True)
name: Mapped[str] = mapped_column(String(255), nullable=False, unique=True)
bio: Mapped[str] = mapped_column(String(512), nullable=False)
books: Mapped[list["Book"]] = relationship(
"Book", back_populates="author"
)


class Book(Base):
__tablename__ = "books"

id: Mapped[int] = mapped_column(primary_key=True, index=True)
title: Mapped[str] = mapped_column(String(255), nullable=False)
summary: Mapped[str] = mapped_column(String(255), nullable=False)
publication_date: Mapped[datetime.date] = mapped_column(Date, nullable=False)
author_id: Mapped[int] = mapped_column(
ForeignKey("authors.id"), nullable=False
)
author: Mapped["Author"] = relationship(
"Author", back_populates="books"
)
69 changes: 69 additions & 0 deletions main.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
from typing import Annotated, Generator
from sqlalchemy import select
from fastapi import FastAPI, Depends, HTTPException
from sqlalchemy.orm import Session
import crud
import schemas
from db.database import SessionLocal
from db.models import Book

app = FastAPI()
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The application is missing the logic to create the database tables, which is a key requirement. You'll need to import engine from your database setup and Base from your models, and then call Base.metadata.create_all(bind=engine) to ensure the tables exist before the API starts.



def get_db() -> Generator[Session, None, None]:
db = SessionLocal()

try:
yield db
finally:
db.close()


@app.get("/books/", response_model=list[schemas.BookListSchema])
def read_cheese_types(
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function name read_cheese_types does not reflect what the function does (reading books). A more descriptive name, such as read_books, would significantly improve the code's readability and maintainability.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function name, read_cheese_types, seems to be a leftover from a template or example. It's a bit confusing for an endpoint that fetches books. Consider renaming it to something more descriptive, like read_books, to match the endpoint's functionality.

db: Annotated[Session, Depends(get_db)],
author_id: int | None = None,
skip: int | None = None,
limit: int | None = None,
):
return crud.get_all_books(
db=db,
author_id=author_id,
skip=skip,
limit=limit
)


@app.post("/books/", response_model=schemas.BookSchemaBase)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The response_model is set to schemas.BookSchemaBase, which does not include the book's id. When a resource is created, the API response should typically include the full resource, including its unique identifier. Consider using a schema that includes the id field.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The response_model is set to schemas.BookSchemaBase, which doesn't include the id of the book. It's a common practice in REST APIs to return the full resource, including its unique identifier, after it's been created.

Consider changing this to schemas.BookListSchema to ensure the id of the newly created book is included in the response.

def create_book_route(
book: schemas.BookCreateSchema,
db: Session = Depends(get_db)
):
return crud.create_book(db=db, book=book)


@app.get("/authors/", response_model=list[schemas.AuthorListSchema])
def get_authors(
db: Annotated[Session, Depends(get_db)],
skip: int | None = None,
limit: int | None = None,
):
return crud.get_authors_list(db=db, skip=skip, limit=limit)


@app.post("/authors/", response_model=schemas.AuthorSchemaBase)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to the book creation endpoint, the response_model here (schemas.AuthorSchemaBase) does not include the new author's id. It's a best practice to return the id of a newly created resource. Using a schema that includes the id would be more conventional for a REST API.

def create_author_route(
author: schemas.AuthorCreateSchema,
db: Session = Depends(get_db)
):
return crud.create_author(db=db, author=author)


@app.get("/authors/{author_id}/", response_model=schemas.AuthorListSchema)
def get_authors_by_author_id(
db: Annotated[Session, Depends(get_db)],
author_id: int,
):
return crud.get_author_by_id(
db=db, author_id=author_id
)
4 changes: 4 additions & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
fastapi
sqlalchemy
alembic
uvicorn
37 changes: 37 additions & 0 deletions schemas.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import datetime
from pydantic import BaseModel


class BookSchemaBase(BaseModel):
title: str
summary: str
publication_date: datetime.date
author_id: int

class Config:
from_attributes = True


class BookCreateSchema(BookSchemaBase):
pass


class BookListSchema(BookSchemaBase):
id: int


class AuthorSchemaBase(BaseModel):
name: str
bio: str

class Config:
from_attributes = True


class AuthorCreateSchema(AuthorSchemaBase):
pass


class AuthorListSchema(AuthorSchemaBase):
id: int
books: list[BookListSchema] = []