Solution#656
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your implementation is mostly solid, but two critical issues prevent approval. First, the database tables are never created—CHECKLIST ITEM #17 explicitly requires calling Base.metadata.create_all(bind=engine) in main.py before the app starts, without which you'll get 'no such table' errors at runtime. Second, the GET /books/ endpoint is missing the author_id query parameter required by CHECKLIST ITEM #6 for filtering books by author. While crud.py has the functionality implemented, the FastAPI endpoint doesn't expose it to users. Add author_id: int = None as a query parameter to read_books and pass it through to crud.get_books(). Once these two issues are addressed, your solution should be complete.
✨ 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
|
|
||
| engine = create_engine( | ||
| SQLALCHEMY_DATABASE_URL, | ||
| connect_args={"check_same_thread": False}, |
There was a problem hiding this comment.
Missing table creation. The task requirement #17 explicitly states to 'Create database tables using the SQLAlchemy models'. This line should be added to initialize the database on app startup.
| @app.get("/books/", response_model=List[BookRead]) | ||
| def read_books(skip: int = 0, limit: int = 10, db: Session = Depends(get_db)): | ||
| return crud.get_books(db, skip=skip, limit=limit) |
There was a problem hiding this comment.
The read_books endpoint is missing the author_id query parameter. According to checklist item #6, you need to filter books by author ID. Add author_id: int = None as a query parameter and pass it to crud.get_books.
|
|
||
| @app.get("/books/", response_model=List[BookRead]) | ||
| def read_books(skip: int = 0, limit: int = 10, db: Session = Depends(get_db)): | ||
| return crud.get_books(db, skip=skip, limit=limit) |
There was a problem hiding this comment.
Missing database table creation. The task requires creating database tables using the SQLAlchemy models (checklist item #17). Add from database import Base, engine and call Base.metadata.create_all(bind=engine) before the app runs, typically at module level or using a startup event.
| database.db | ||
| database.py | ||
| .idea | ||
|
|
There was a problem hiding this comment.
Missing table creation: The task requires creating database tables using SQLAlchemy models (checklist item #17), but Base.metadata.create_all(bind=engine) is never called. Add this line to create the tables when the app starts.
| @@ -0,0 +1,41 @@ | |||
| from sqlalchemy import select | |||
| from models import Author, Book | |||
| from schemas import AuthorCreate, BookCreate | ||
|
|
||
|
|
||
| def get_author(db, author_id: int): |
There was a problem hiding this comment.
Missing file: schemas.py is not provided but is required. The task specification explicitly requires this file (CHECKLIST ITEM #14). Line 6 of main.py imports from this file: from schemas import AuthorRead, AuthorCreate, BookCreate, BookRead
| ) | ||
| db.add(db_book) | ||
| db.commit() |
There was a problem hiding this comment.
Missing feature: This endpoint does not support filtering books by author ID as required by CHECKLIST ITEM #6. The function should accept an optional author_id: int = None query parameter and pass it to crud.get_books().
|
|
||
| class AuthorCreate(BaseModel): | ||
| name: str | ||
| bio: str |
There was a problem hiding this comment.
Missing table creation: The task requires creating database tables using SQLAlchemy models (checklist item #17). Add Base.metadata.create_all(bind=engine) to create the tables when the application starts.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
The implementation is complete and fully satisfies all task requirements. Both critical issues from the previous review have been successfully resolved: (1) database tables are now created at startup via Base.metadata.create_all(bind=engine) in main.py, and (2) the GET /books/ endpoint properly exposes the author_id query parameter for filtering books by author. All five required files are present with correct structure, SQLAlchemy models include all specified fields with proper relationships, Pydantic schemas validate input/output correctly, CRUD functions are implemented, and all API endpoints (POST/GET /authors/, GET /authors/{id}, POST/GET /books/) function as specified with pagination support. Your solution is approved and ready for use.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
No description provided.