feat: migrate list and series seeds json endpoints to FastAPI#12753
feat: migrate list and series seeds json endpoints to FastAPI#12753harshgupta2125 wants to merge 9 commits into
Conversation
github-project-automation
Bot
moved this to Waiting Review/Merge from Staff
in Ray's Project
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Pull request overview
This PR migrates list/series seed JSON endpoints from legacy web.py handling into the FastAPI lists router while reusing the existing legacy seed mutation logic.
Changes:
- Adds FastAPI GET/POST routes for list and series seed JSON endpoints.
- Adds helper methods for seed lookup and update handling.
- Updates the shared legacy seed lookup helper to use the request-context
site.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
openlibrary/fastapi/lists.py |
Adds FastAPI routes and helpers for list/series seeds GET and POST operations. |
openlibrary/plugins/openlibrary/lists.py |
Updates get_list_seeds to use the FastAPI-compatible request context site accessor. |
Sanket17052006
left a comment
Sanket17052006
left a comment
There was a problem hiding this comment.
Hey @harshgupta2125, this seems to be heading in the right direction, I haven't thoroughly tested but these are few suggestions
- You should add the deprecated decorator in the legacy endpoint
- Also a test file would be great as the other migrations follows the same pattern.
harshgupta2125
force-pushed
the
migrate-list-seeds-json
branch
from
4af88c1 to
837e914
Compare
for more information, see https://pre-commit.ci
@Sanket17052006, thanks for taking a look and for the guidance! I've added the @deprecated("migrated to fastapi") decorator to the legacy list_seeds class in openlibrary/plugins/openlibrary/lists.py. I also added a complete test suite in openlibrary/tests/fastapi/test_list_seeds.py covering the GET (200/404) and POST (auth/permissions) routes to match the pattern of the other migrations. |
| "entries": seeds, | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
The deprecated decorator should be added here I don't see it though! Hope it helps :)
There was a problem hiding this comment.
I had the file staged locally but accidentally dropped it during a git commit --amend while I was polishing up the test suite.
I just pushed a new commit that officially adds the @deprecated("migrated to fastapi")
| content_type = "text/yaml" | ||
|
|
||
|
|
||
| deprecated("migrated to fastapi") |
There was a problem hiding this comment.
Is there a reason for this here? Also the get_list_seeds is still being used in the new FastAPI endpoint. I think it would be a good idea to look at other lists migration PR, reading similar PR's helps me to get better at these migrations, hope the same for you!
There was a problem hiding this comment.
Total typo missing the @ and putting it in the wrong spot.
You're completely right about get_list_seeds—I'm only deprecating the legacy list_seeds routing class, not the helper function since FastAPI still needs it. I've pushed the fix. Thanks for the guidance.
harshgupta2125
force-pushed
the
migrate-list-seeds-json
branch
from
0acc5d8 to
f74f07d
Compare
github-actions
Bot
added
the
Needs: Response
Closes #12487
refactor: This PR migrates the legacy web.py JSON endpoints for fetching and updating List and Series "seeds" to the modern FastAPI architecture.
Technical
/people/{user}/lists/{olid}/seeds.json,/people/{user}/series/{olid}/seeds.json, and the public equivalents) to explicit FastAPI route definitions with native 404/403 HTTP Exception handling.POSTendpoints, the complex seed mutation logic is preserved by routing the safely extractedaddandremovearrays directly into_LegacyListSeeds.process_seeds_update().payload: Annotated[dict, Body(...)]to safely consume arbitrary incoming JSON without forcing strict Pydantic model evaluation on the legacy payload shape.GETreturn type hints strictly todict(matching the legacy{"entries": [...], "size": ...}output shape) to resolveResponseValidationErrorcrashes during outbound serialization.web.ctx.sitedependencies for the modernsite.get()in the legacyget_list_seedsfallback to preventThreadedDictaccess issues.Testing
GETandPOSTroutes pass viadocker compose exec web python -m pytest openlibrary/tests/fastapi/ -vhttp://localhost:8080/people/openlibrary/lists/OL3L/seeds.json. Verify the JSON correctly outputs the list seeds.http://localhost:8080/docsPOST /people/{username}/lists/{olid}/seeds.jsonendpoint.{"add": ["/works/OL123W"], "remove": []}) to verify the mutation succeeds and respects theDepends(require_authenticated_user)lock.Screenshot
N/A - Backend API migration only.
Stakeholders
@RayBB