Skip to content

add patch, delete, get methods#5

Open
gelono wants to merge 2 commits into
p1v2:mainfrom
gelono:main
Open

add patch, delete, get methods#5
gelono wants to merge 2 commits into
p1v2:mainfrom
gelono:main

Conversation

@gelono

@gelono gelono commented May 16, 2023

Copy link
Copy Markdown

Home work 10. Zadniprovskyi Oleg

Comment thread app/routers/books.py Outdated
item = session.query(Book).filter(Book.id == book_id).first()
except SQLAlchemyError as e:
print(e)
return {"message": "error while working with database"}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

pls return response with the correct HTTP status here.

Comment thread app/routers/books.py Outdated
return BookModel.from_orm(item)
else:
raise HTTPException(
status_code=status.HTTP_204_NO_CONTENT,

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

404 should be returned there. 204 is HTTP status code for the successful deletion

Comment thread app/routers/books.py Outdated
)


@router.delete("/{book_id}", response_model=BookModel, status_code=200)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

status_code there should be 204

@gelono

gelono commented May 16, 2023

Copy link
Copy Markdown
Author

I did a fix and commit to my repo. Should I need to do the new PR?

@p1v2

p1v2 commented May 16, 2023 via email

Copy link
Copy Markdown
Owner

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants