-
Notifications
You must be signed in to change notification settings - Fork 441
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Develop #409
base: main
Are you sure you want to change the base?
Develop #409
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job on meeting the core requirements of the task! 🎉 While there are a few minor improvements suggested, such as refining relationship attribute names and adding return types for clarity, these do not impact the overall functionality of your code. Keep up the good work, and consider the feedback for future enhancements! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
@@ -0,0 +1,79 @@ | |||
from logging.config import fileConfig |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The import statement from logging.config import fileConfig
is missing a newline character at the end. This could lead to syntax errors. Ensure that each import statement is on its own line.
def create_author(db: Session, author: AuthorCreate): | ||
"""Creates a new author to the database.""" | ||
db_author = models.DBAuthor( | ||
name=author.name, | ||
bio=author.bio, | ||
) | ||
db.add(db_author) | ||
db.commit() | ||
db.refresh(db_author) | ||
|
||
return db_author |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The create_author
function should specify a return type in its signature to improve code clarity and type checking. Consider adding -> models.DBAuthor
to the function definition.
def create_book(db: Session, book: BookCreate): | ||
"""Creates a new book, connected to the author.""" | ||
|
||
db_book = models.DBBook( | ||
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The create_book
function should specify a return type in its signature to improve code clarity and type checking. Consider adding -> models.DBBook
to the function definition.
skip: int, | ||
limit: int, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The skip
and limit
parameters in the read_books
function should have default values and validation similar to the read_authors
function. Consider using Query
with ge=0
for skip
and gt=0
for limit
to ensure valid input.
id = Column(Integer, primary_key=True, index=True) | ||
name = Column(String(255), nullable=False, unique=True) | ||
bio = Column(String(511)) | ||
books = relationship("DBBook", back_populates="authors") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The relationship
attribute books
in the DBAuthor
class should be named book
to match the back_populates
attribute in the DBBook
class, which is authors
. This ensures consistency and clarity in the relationship mapping.
publication_date = Column(Date) | ||
author_id = Column(Integer, ForeignKey("authors.id")) | ||
|
||
authors = relationship("DBAuthor", back_populates="books") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The relationship
attribute authors
in the DBBook
class should be named author
to better reflect that it represents a single author related to the book. This will also align with the back_populates
attribute in the DBAuthor
class.
No description provided.