Skip to content

feat: migrate list and series seeds json endpoints to FastAPI#12753

Open
harshgupta2125 wants to merge 9 commits into
internetarchive:masterfrom
harshgupta2125:migrate-list-seeds-json
Open

feat: migrate list and series seeds json endpoints to FastAPI#12753
harshgupta2125 wants to merge 9 commits into
internetarchive:masterfrom
harshgupta2125:migrate-list-seeds-json

Conversation

@harshgupta2125
Copy link
Copy Markdown
Contributor

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

  • FastAPI Routing & Validation: Migrated all 4 route variants (/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.
  • Legacy Mutator Preservation: For POST endpoints, the complex seed mutation logic is preserved by routing the safely extracted add and remove arrays directly into _LegacyListSeeds.process_seeds_update().
  • Payload Handling: Utilized payload: Annotated[dict, Body(...)] to safely consume arbitrary incoming JSON without forcing strict Pydantic model evaluation on the legacy payload shape.
  • Schema Output Validation: Set the GET return type hints strictly to dict (matching the legacy {"entries": [...], "size": ...} output shape) to resolve ResponseValidationError crashes during outbound serialization.
  • Context & Helpers: Swapped legacy web.ctx.site dependencies for the modern site.get() in the legacy get_list_seeds fallback to prevent ThreadedDict access issues.

Testing

  • Unit Testing: Verified all GET and POST routes pass via docker compose exec web python -m pytest openlibrary/tests/fastapi/ -v
  • Verify GET: Navigate to http://localhost:8080/people/openlibrary/lists/OL3L/seeds.json. Verify the JSON correctly outputs the list seeds.
  • Verify POST:
    1. Navigate to the interactive docs: http://localhost:8080/docs
    2. Authenticate your local session.
    3. Expand the POST /people/{username}/lists/{olid}/seeds.json endpoint.
    4. Execute a test payload (e.g., {"add": ["/works/OL123W"], "remove": []}) to verify the mutation succeeds and respects the Depends(require_authenticated_user) lock.

Screenshot

N/A - Backend API migration only.

Stakeholders

@RayBB

Copilot AI review requested due to automatic review settings May 16, 2026 16:50
@github-project-automation github-project-automation Bot moved this to Waiting Review/Merge from Staff in Ray's Project May 16, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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.

Comment thread openlibrary/fastapi/lists.py
Comment thread openlibrary/fastapi/lists.py
Comment thread openlibrary/fastapi/lists.py Outdated
Comment thread openlibrary/fastapi/lists.py Outdated
Comment thread openlibrary/fastapi/lists.py
Copy link
Copy Markdown
Contributor

@Sanket17052006 Sanket17052006 left a comment

Choose a reason for hiding this comment

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

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 harshgupta2125 force-pushed the migrate-list-seeds-json branch from 4af88c1 to 837e914 Compare May 16, 2026 17:41
@harshgupta2125
Copy link
Copy Markdown
Contributor Author

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.

@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,
}


Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The deprecated decorator should be added here I don't see it though! Hope it helps :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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 harshgupta2125 force-pushed the migrate-list-seeds-json branch from 0acc5d8 to f74f07d Compare May 16, 2026 19:01
@github-actions github-actions Bot added the Needs: Response Issues which require feedback from lead label May 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs: Response Issues which require feedback from lead

Projects

Status: Waiting Review/Merge from Staff

Development

Successfully merging this pull request may close these issues.

Migrate lists endpoints to fastapi

4 participants