Skip to content

Better World Books OPDS feed importer#12852

Draft
ronibhakta1 wants to merge 6 commits into
internetarchive:masterfrom
ronibhakta1:12844/feature/tbp-ingestion-cron
Draft

Better World Books OPDS feed importer#12852
ronibhakta1 wants to merge 6 commits into
internetarchive:masterfrom
ronibhakta1:12844/feature/tbp-ingestion-cron

Conversation

@ronibhakta1

Copy link
Copy Markdown
Contributor

Closes #12844

refactor BWB script

Technical

Testing

Screenshot

Stakeholders

@cdrini @mekarpeles

Copilot AI review requested due to automatic review settings June 4, 2026 05:57

Copilot AI left a comment

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.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds Trusted Book Provider (TBP) infrastructure (feed registry + acquisitions) and extends the Better World Books OPDS importer to record purchase acquisitions and persist incremental progress via the registry.

Changes:

  • Introduces acquisitions + tbp_feed_registry tables and corresponding Python model/service layers (Acquisition, FeedRegistry) with tests.
  • Extends scripts/bwb_opds_imports.py to extract buy URLs/acquisition data, upsert acquisitions for existing editions, and use tbp_feed_registry.last_updated as the primary incremental cursor.
  • Expands OPDS importer tests to cover acquisition extraction, acquisition upsert mapping, and registry cursor normalization.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
scripts/tests/test_bwb_opds_imports.py Adds coverage for acquisition extraction/collection, acquisition upserts, and registry cursor normalization.
scripts/bwb_opds_imports.py Extracts buy URL/acquisition data, collects acquisitions during crawl, upserts to DB, and uses the TBP registry cursor.
openlibrary/tests/core/test_tbp.py Adds unit tests for FeedRegistry CRUD/idempotent registration/advance.
openlibrary/tests/core/test_acquisitions.py Adds unit tests for acquisition lookup, upsert behavior, and merge remapping helpers.
openlibrary/core/tbp.py Implements the TBP feed registry interface.
openlibrary/core/schema.sql Adds acquisitions and tbp_feed_registry tables + indexes.
openlibrary/core/acquisitions.py Implements the acquisitions table interface, including upsert and edition-merge remapping.

Comment thread openlibrary/core/acquisitions.py Outdated
Comment thread openlibrary/core/acquisitions.py Outdated
Comment thread scripts/bwb_opds_imports.py Outdated
Comment thread scripts/bwb_opds_imports.py Outdated
Comment thread scripts/bwb_opds_imports.py Outdated
Comment thread scripts/bwb_opds_imports.py Outdated
Comment thread scripts/bwb_opds_imports.py
Comment thread openlibrary/core/tbp.py
@mekarpeles

Copy link
Copy Markdown
Member

Thanks for the PR, @ronibhakta1!

🤖 Copilot has been assigned for an initial review.

The linked issue (#12844) hasn't been triaged yet — triage happens on Mondays and Fridays. There are currently 74 open non-draft PRs ahead of yours.

Possible improvements for this PR

  • PR description is incomplete — the Technical, Testing, and Screenshot sections are unfilled. For future PRs, please follow the PR template and provide details. Here's a draft based on the diff and linked issue:

    This PR implements the first three subtasks of the TBP ingestion pipeline (#12844): it introduces a tbp_feed_registry table and FeedRegistry class for tracking provider feeds, an acquisitions table and Acquisition class for storing per-edition provider metadata, and extends bwb_opds_imports.py to extract acquisition records from the BWB OPDS feed and upsert them into the new table. Unit tests are included for all three new modules.

  • No proof of testing — the Testing section is empty and no test output is shown. A brief description of how you ran or validated the changes (e.g. running the test suite locally, a dry-run of the importer) would help reviewers move faster. See how to attach files if you'd like to include output.

PR triage checklist (maintainers / Pam)
  • PR description — not empty; explains what the change does and how to verify it
  • References an issue — PR body contains a #NNN reference
    • Linked issue is triaged — has a Priority: * label (not just Needs: Triage)
    • Linked issue is assigned — has at least one assignee
  • Commit history clean — no WIP/fixup/conflict noise; commit messages are meaningful (note: history includes a merge commit from 12844/feature/tbp-feed-registry; consider rebasing)
  • CI passing — no failing check-runs
  • Test cases present — if the change touches substantive logic, test coverage exists or is explained
  • Proof of testing — PR body includes a description of what was tested, a screenshot, or a video

Note

This comment was automatically generated by Pam, Open Library's Project AI Manager, on behalf of @mekarpeles. Pam is designed to provide status visibility, perform basic project management functions and relevant codebase research, and provide actionable feedback so contributors aren't left waiting.

@mekarpeles mekarpeles added the Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] label Jun 4, 2026
@github-actions github-actions Bot removed the Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] label Jun 4, 2026
@cdrini

cdrini commented Jun 20, 2026

Copy link
Copy Markdown
Collaborator

Pushed up CR feedback from #12851 ; apologies likely causes some issues with some of the other code! Continuing to review/push up fixes over the weekend. I will review #12846 next, and push up any small CR fixes here, before reviewing this one and giving everything a test.

@cdrini
cdrini force-pushed the 12844/feature/tbp-ingestion-cron branch from 3e2c8e3 to a26abe2 Compare June 21, 2026 14:28
- Add pydantic typing for BWB OPDS feed
- Remove logic for writing to file ; write everything to DB now
- Update to handle new Acquistion local_id

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@cdrini
cdrini force-pushed the 12844/feature/tbp-ingestion-cron branch from d32b819 to 911817f Compare June 22, 2026 22:12
@cdrini
cdrini marked this pull request as draft June 22, 2026 22:12
@cdrini cdrini added the Priority: 1 Do this week, receiving emails, time sensitive, . [managed] label Jun 22, 2026
@cdrini cdrini changed the title 12844/feature/tbp ingestion cron Better World Books OPDS feed importer Jun 22, 2026
@mekarpeles mekarpeles added Priority: 2 Important, as time permits. [managed] and removed Priority: 1 Do this week, receiving emails, time sensitive, . [managed] labels Jul 6, 2026
@mekarpeles mekarpeles added this to the Sprint 2026-07 milestone Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Priority: 2 Important, as time permits. [managed]

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Open Library OPDS Registry. Trusted Book Providers (TBP) Ingestion Pipeline.

4 participants