Implement FastAPI Library Management API with CRUD operations#639
Implement FastAPI Library Management API with CRUD operations#6393883IZ wants to merge 4 commits into
Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Changes are needed because there are a couple of critical issues that will prevent the app from running correctly and from meeting the task’s core requirements. In app/main.py near the bottom of the file, the books listing endpoint (def read...) is left incomplete, which introduces a syntax error and means there is no working endpoint to list books with pagination and optional author_id filtering as required; you should implement something like def read_books(skip: int = 0, limit: int = 10, author_id: Optional[int] = None, db: Session = Depends(get_db)) and return a paginated query. In app/schemas.py, both the Book and Author models use class Config: from_attributes = True, which is not a valid way to enable ORM mode for FastAPI; update this to class Config: orm_mode = True (Pydantic v1) or to a class-level model_config = {"from_attributes": True} (Pydantic v2), otherwise returning SQLAlchemy objects will fail at runtime. The CRUD helper that retrieves books by author currently returns all matches without pagination, so it should be extended to accept skip and limit parameters and apply .offset(skip).limit(limit) so the listing endpoint can satisfy the pagination requirement. The endpoint that creates a book for a specific author does not verify that the author exists before creation; add a call to something like crud.get_author(db, author_id=author_id) and raise HTTPException(status_code=404, detail="Author not found") if it returns None to avoid orphaned references. As a smaller robustness issue, in app/schemas.py the field books: List[Book] = [] on the Author schema uses a mutable default; switch it to books: List[Book] = Field(default_factory=list) (and import Field from pydantic) to avoid shared state across instances. Once you complete the books endpoint, fix the Pydantic configuration, and wire pagination/author checks into your CRUD and routes, your solution will be much closer to fully correct and production-ready. I encourage you to treat these gaps as a chance to practice end-to-end API design—your models and general structure are solid, so focusing on these details will get you to a complete, working implementation.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| author_id = Column(Integer, ForeignKey("authors.id")) | ||
| author = relationship("Author", back_populates="books") |
There was a problem hiding this comment.
This helper returns all books for an author without pagination. The task requires listing books with pagination (skip, limit); consider adding skip and limit parameters and applying offset/limit here (or ensure the endpoint uses a CRUD function that supports pagination).
| @app.get("/books/", response_model=list[schemas.Book]) | ||
| def read |
There was a problem hiding this comment.
The books listing endpoint is unfinished — def read is incomplete and causes a syntax error. Implement the endpoint to return a paginated list of books and support filtering by author_id (e.g. signature like def read_books(skip: int = 0, limit: int = 10, author_id: Optional[int] = None, db: Session = Depends(get_db))).
|
|
||
| @app.get("/authors/", response_model=list[schemas.Author]) |
There was a problem hiding this comment.
The CRUD helper get_books_by_author does not accept skip and limit parameters, but the requirements expect retrieving books with pagination and filtering by author. Consider adding skip and limit args and applying .offset(skip).limit(limit) to the query so the endpoint can return paginated results.
| def create_book_for_author(author_id: int, book: schemas.BookCreate, db: Session = Depends(get_db)): | ||
| return crud.create_book(db=db, book=book, author_id=author_id) |
There was a problem hiding this comment.
The endpoint that creates a book for a given author doesn't check whether the author exists before creating the book. To meet expected behavior and avoid orphaned references, call crud.get_author and raise HTTP 404 if the author is not found before creating the book.
| class Config: | ||
| from_attributes = True # ✅ заміна orm_mode |
There was a problem hiding this comment.
Issue: Config inside Book uses from_attributes = True, which is not a valid Pydantic setting in this form. Fix: For Pydantic v1 use class Config: orm_mode = True. For Pydantic v2 use model_config = {"from_attributes": True} at the model class level. This is required so FastAPI can serialize SQLAlchemy ORM objects to the response model.
| class Config: | ||
| from_attributes = True # ✅ заміна orm_mode |
There was a problem hiding this comment.
Issue: Config inside Author uses from_attributes = True as well (same problem). Fix: Replace with orm_mode = True (Pydantic v1) or model_config = {"from_attributes": True} (Pydantic v2) so Author ORM objects serialize correctly in responses.
| class Author(AuthorBase): | ||
| id: int | ||
| books: List[Book] = [] |
There was a problem hiding this comment.
Using a mutable default books: List[Book] = [] can cause shared-state bugs across instances. Fix: Use books: List[Book] = Field(default_factory=list) or books: List[Book] = [] but preferably from pydantic import Field and default_factory=list to create a new list per instance.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there is a schema ↔ model mismatch that will cause runtime errors in core API functionality.
Critical issues that must be fixed before this can pass:
- In
app/models.py, theBookSQLAlchemy model defines fieldssummaryandpublication_date, but inapp/schemas.pyyour PydanticBook/BookCreateschemas usedescriptionand do not includepublication_date. Whencreate_bookinapp/crud.pycalls something likemodels.Book(**book.dict(), author_id=author_id), this will raiseTypeError: __init__() got an unexpected keyword argument 'description'and will also fail to provide the requiredpublication_dateto the ORM model. To fix this, either rename the schema fielddescription→summaryand addpublication_date: date(imported fromdatetime) to the relevant Book schemas, or explicitly map fields in the CRUD helper, e.g.models.Book(title=book.title, summary=book.description, publication_date=book.publication_date, author_id=author_id). - Your schemas currently use
model_config = {"from_attributes": True}which is the Pydantic v2 way to enable ORM mode. If your environment is using Pydantic v1, FastAPI will not serialize ORM objects correctly and responses will fail; in that case you must instead add an innerclass Config: orm_mode = Trueto the Pydantic models. Please confirm the Pydantic version and adjust the configuration (or pin the version in your dependencies) so ORM responses work reliably.
As minor improvements (not blockers), you could add explicit status_code=201 to your create endpoints and consider catching IntegrityError on commits to return clear 4xx errors instead of generic 500s. Overall, your project structure, endpoints, and use of FastAPI + SQLAlchemy look solid; once you align the Book schemas with the ORM model and ensure the correct Pydantic ORM configuration, your solution should work as a functional library management API.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| def read_author(author_id: int, db: Session = Depends(get_db)): | ||
| db_author = crud.get_author(db, author_id=author_id) | ||
| if db_author is None: | ||
| raise HTTPException(status_code=404, detail="Author not found") |
There was a problem hiding this comment.
The CRUD helper uses models.Book(**book.dict(), author_id=author_id) which will fail if the Pydantic BookCreate schema keys don't match the SQLAlchemy model attributes. Your SQLAlchemy Book model defines summary and publication_date but the provided Book schema uses description and lacks publication_date. Align the names and fields (either change schema to provide summary and publication_date, or change the model to accept description) or explicitly map fields when creating the model instance (e.g. models.Book(title=book.title, summary=book.description, publication_date=book.publication_date, author_id=author_id)).
| __tablename__ = "authors" | ||
|
|
||
| id = Column(Integer, primary_key=True, index=True) | ||
| name = Column(String, unique=True, index=True) | ||
| bio = Column(String) | ||
| books = relationship("Book", back_populates="author") |
There was a problem hiding this comment.
The Book schema defines description but your SQLAlchemy Book model defines summary (and the schema doesn't include publication_date). This mismatch will cause TypeError: __init__() got an unexpected keyword argument 'description' when the code does models.Book(**book.dict(), author_id=...). Align the names and fields: either rename description -> summary and add publication_date: date to the schema (import date from datetime), or explicitly map schema fields to model fields when creating the Book instance.
| summary = Column(String) | ||
| publication_date = Column(Date) |
There was a problem hiding this comment.
The SQLAlchemy Book model declares summary and publication_date fields. Make sure your Pydantic create/update schemas expose matching fields (names and types). Right now the mismatch between summary (model) and description (schema) will break the CRUD creation path.
| class BookBase(BaseModel): | ||
| title: str | ||
| description: Optional[str] = None |
There was a problem hiding this comment.
The Book schema uses description but the SQLAlchemy Book model defines the field as summary (and also has publication_date). This mismatch will cause models.Book(**book.dict(), ...) to raise a TypeError or leave required fields empty. Rename description to summary and add publication_date to the schema (or explicitly map fields when creating the Book in CRUD).
| @@ -0,0 +1,33 @@ | |||
| from typing import List, Optional | |||
| from pydantic import BaseModel, Field | |||
There was a problem hiding this comment.
Because the DB model includes publication_date, add that field to your Book schemas (e.g. publication_date: Optional[date] = None) and import date from datetime at the top of this file. Without this the create-book flow cannot provide the model's required attribute.
| id: int | ||
| author_id: int | ||
|
|
||
| model_config = {"from_attributes": True} |
There was a problem hiding this comment.
You use model_config = {"from_attributes": True} which is the Pydantic v2 way to allow parsing ORM objects. If your environment uses Pydantic v1 you must instead set class Config: orm_mode = True on the Pydantic models. Confirm the Pydantic version and adjust accordingly so FastAPI can return SQLAlchemy ORM objects via these schemas.
| id: int | ||
| books: List[Book] = Field(default_factory=list) | ||
|
|
||
| model_config = {"from_attributes": True} |
There was a problem hiding this comment.
Same Pydantic-compatibility note for the Author schema: model_config = {"from_attributes": True} is v2-style. If you run under Pydantic v1 change this to class Config: orm_mode = True to ensure ORM objects are serialized correctly.
…nfig, update .gitignore
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution as an educational override so you can keep progressing while you continue learning, but there are a few important improvements to address. You successfully set up the FastAPI + SQLAlchemy + SQLite stack, defined Author and Book models and schemas that now match (including summary and publication_date), and implemented all required endpoints for creating and listing authors and books with pagination and author filtering. A critical follow-up is to confirm your Pydantic version and ORM configuration: in app/schemas.py (around the Author and Book Pydantic models, roughly lines 10–60) you use model_config = {"from_attributes": True}, which only works on Pydantic v2; if your environment is v1 you should instead add an inner class Config: orm_mode = True so FastAPI can serialize ORM objects correctly. Another high-priority fix is in your create operations (e.g., create_author in app/main.py and create_book in app/crud.py, around the db.add(...) / db.commit() calls): wrap db.commit() in a try/except sqlalchemy.exc.IntegrityError block, call db.rollback() on error, and return an appropriate 4xx HTTPException (for example when the unique Author.name constraint is violated) instead of letting a 500 occur. In create_book (in app/crud.py, near the models.Book(**book.dict(), author_id=author_id) line), be mindful that book.dict() is a Pydantic v1 idiom and that this unpacking relies on schema fields matching the ORM model exactly; either switch to book.model_dump() for Pydantic v2 or explicitly pass the fields like models.Book(title=..., summary=..., publication_date=..., author_id=author_id) to avoid fragile coupling. As a minor REST improvement, consider adding status_code=201 to your POST endpoints in app/main.py so that successful resource creation is communicated with the conventional HTTP status. Overall, your structure and functionality are on the right track—fixing these Pydantic and error-handling details will make your API much more robust and is an excellent learning step.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
|
|
||
| def get_db(): | ||
| db = SessionLocal() | ||
| try: |
There was a problem hiding this comment.
In create_author you commit immediately without handling possible DB integrity errors (for example duplicate name unique constraint). Consider catching sqlalchemy.exc.IntegrityError and returning a controlled 4xx response instead of letting a 500 bubble up.
| def read_author(author_id: int, db: Session = Depends(get_db)): | ||
| db_author = crud.get_author(db, author_id=author_id) | ||
| if db_author is None: | ||
| raise HTTPException(status_code=404, detail="Author not found") |
There was a problem hiding this comment.
This line uses **book.dict() to construct the ORM model. If you target Pydantic v2 prefer book.model_dump() for dumping fields; also ensure the Pydantic BookCreate fields exactly match the ORM Book constructor (they appear to match now). If you ever change schema field names, map them explicitly here to avoid unexpected keyword errors.
| finally: | ||
| db.close() | ||
|
|
||
| @app.post("/authors/", response_model=schemas.Author) |
There was a problem hiding this comment.
This endpoint (and others using response_model) relies on Pydantic ORM-mode to serialize SQLAlchemy objects. Your schemas use model_config = {"from_attributes": True} (Pydantic v2). Confirm your environment is using Pydantic v2; if it uses Pydantic v1 instead, add class Config: orm_mode = True to the Pydantic models so FastAPI can serialize ORM objects.
| raise HTTPException(status_code=404, detail="Author not found") | ||
| return db_author | ||
|
|
||
| @app.post("/authors/{author_id}/books/", response_model=schemas.Book) |
There was a problem hiding this comment.
Minor: consider adding status_code=201 to create endpoints (e.g. this POST) so responses follow the usual REST convention for resource creation.
| from sqlalchemy.orm import Session | ||
| from app import models, schemas | ||
|
|
||
| def get_author(db: Session, author_id: int): |
There was a problem hiding this comment.
The app imports schemas and uses Pydantic response_model throughout. Your schemas.py uses model_config = {"from_attributes": True} (Pydantic v2 style). This will only work if the runtime uses Pydantic v2. Please confirm the Pydantic version. If the environment uses Pydantic v1, add an inner class Config: orm_mode = True to the Pydantic models (or pin Pydantic to v2). This is required so FastAPI can serialize ORM objects for the endpoints declared below.
|
|
||
| class BookBase(BaseModel): | ||
| title: str | ||
| summary: Optional[str] = None |
There was a problem hiding this comment.
The name column is declared unique=True which is correct per the requirements, but this means inserts can raise DB integrity errors (duplicate names). Make sure your CRUD create_author catches and handles sqlalchemy.exc.IntegrityError and returns a suitable 4xx response (e.g. 409) instead of letting a 500 be returned.
| id: int | ||
| author_id: int | ||
|
|
||
| model_config = {"from_attributes": True} |
There was a problem hiding this comment.
This enables Pydantic ORM-mode using Pydantic v2 (model_config = {"from_attributes": True}). Confirm your runtime uses Pydantic v2. If it uses Pydantic v1, add class Config: orm_mode = True to the Pydantic models (or pin Pydantic to v2). Also if you switch to Pydantic v2, update CRUD usage to model_dump() where appropriate — book.dict() is a v1 idiom and may not be the recommended approach in v2.
| id: int | ||
| books: List[Book] = Field(default_factory=list) | ||
|
|
||
| model_config = {"from_attributes": True} |
There was a problem hiding this comment.
Same as above for the Author schema: this model_config line is Pydantic v2-specific. Ensure the runtime Pydantic version is compatible or add class Config: orm_mode = True so FastAPI can serialize ORM objects when returning Author instances.
| id = Column(Integer, primary_key=True, index=True) | ||
| title = Column(String, index=True) | ||
| summary = Column(String) | ||
| publication_date = Column(Date) |
There was a problem hiding this comment.
This sets Pydantic ORM behaviour using Pydantic v2 syntax (model_config = {"from_attributes": True}). Confirm your runtime uses Pydantic v2. If the environment uses Pydantic v1, replace this with an inner class Config: orm_mode = True on the model so FastAPI can serialize SQLAlchemy ORM objects correctly.
|
|
||
| id = Column(Integer, primary_key=True, index=True) | ||
| name = Column(String, unique=True, index=True) | ||
| bio = Column(String) |
There was a problem hiding this comment.
publication_date: date is required on BookBase (good — matches the ORM model). Note: your CRUD helper currently calls book.dict() to build the ORM Book. If you adopt Pydantic v2 consider using book.model_dump() in CRUD or otherwise ensure book.dict() remains the intended method for your Pydantic version to avoid subtle incompatibilities.
This pull request delivers the complete implementation of the FastAPI-based Library Management API.
Included:
app/packagedatabase.py) using SQLitefrom_attributes)main.py.gitignoreandrequirements.txtfor environment setupVerification artifacts:
library.dbafter table creationThis PR is ready for mentor review and demonstrates repository hygiene, working API endpoints, and proper environment configuration.