Skip to content

migrate /people/{username}/books/{key}.json to fastapi#11926

Merged
cdrini merged 5 commits intomasterfrom
fastapi/mybooks
Feb 25, 2026
Merged

migrate /people/{username}/books/{key}.json to fastapi#11926
cdrini merged 5 commits intomasterfrom
fastapi/mybooks

Conversation

@RayBB
Copy link
Copy Markdown
Collaborator

@RayBB RayBB commented Feb 25, 2026

  • migrate mybooks to fastapi
  • change import order for merge conflicts

Closes #

Probably will have merge conflicts with #11925

Technical

Testing

Screenshot

Stakeholders

@RayBB RayBB changed the title fastapi/mybooks migrate mybooks to fastapi Feb 25, 2026
@RayBB RayBB changed the title migrate mybooks to fastapi migrate /people/{username}/books/{key}.json to fastapi Feb 25, 2026
Copy link
Copy Markdown
Collaborator

@cdrini cdrini left a comment

Choose a reason for hiding this comment

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

Lgtm! Testing worked well. One blocker with regards to handling invalid cases like invalid username / private reading log.

Note this is a slight breaking change because this endpoint now returns a different error message and returns non-200 status code. Non-200 status codes for error cases is consistent with fastapi's best practices though ; they return eg 422 automatically when the parameters don't match the types.

Comment on lines +60 to +61
page: int = Query(1, ge=1),
limit: int = Query(100, ge=1, le=1000),
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can we use the pagination model to make this api consistent with the others?

@cdrini cdrini merged commit 8b99023 into master Feb 25, 2026
8 checks passed
@cdrini cdrini deleted the fastapi/mybooks branch February 25, 2026 18:21
@RayBB RayBB removed the On Testing label Mar 1, 2026
bhardwajparth51 pushed a commit to bhardwajparth51/openlibrary that referenced this pull request Mar 3, 2026
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