Walk the Overdrive full import over the product list - #3641
Walk the Overdrive full import over the product list#3641jonathangreen wants to merge 3 commits into
Conversation
fetch_book_info_list fused two jobs: walking the update feed's pages and fetching each title's metadata and availability documents. The second job is not specific to the feed -- any enumeration of products needs it -- so it moves to hydrate_products, which takes raw product dictionaries from any source and attaches the per-title documents. fetch_book_info_list now delegates to it, unchanged in behavior. This is also the one place a later change to bulk metadata and availability lookups will land.
The cursor's fault messages ended with the reaper's consequence -- 'Refusing to reap across a gap' -- which was wrong the moment a second consumer appeared: a full import walking the same crawl stops on the same faults but reaps nothing. The reasons now state the arithmetic and nothing else, and each consumer appends its own consequence when it logs; the reaper's lines read as before.
A full import used to sweep the whole collection through the update feed with lastUpdateTime pinned to the epoch. That feed is ordered by update time, which reshuffles on every circulation event anywhere in the collection, so a multi-hour walk over it skips titles the same way the reaper's forward walk did -- and totalItems moves in step, so no count can see it. The import_all path now drives the same CrawlCursor the reaper walks: the dateAdded:asc product list, backwards, ending on a fresh page at offset 0, with the importer hydrating each page's raw products through the split-out hydrate_products. Faults read differently here than in the reaper, because nothing acts on a title's absence. A structural fault ends the crawl but keeps what was imported: the identifier set is still returned, so a child import can use a partial set, while the import timestamp is left unchanged so the incomplete sweep is not recorded as a finished import. A count shortfall at the end is only a warning, and the import finishes. A full-import page queued by the previous release still carries its feed url and is finished the way it was started; the crawl only takes over runs that begin after the deploy.
|
Claude finished @jonathangreen's task in 9m 13s —— View job Review
SummaryThe change is well-shaped: the hydration split is behavior-preserving ( DetailsMinor:
|
Greptile SummaryThe PR moves Overdrive full imports from the mutable update feed to the stable product-list crawl and extracts per-product hydration for reuse.
Confidence Score: 4/5The PR should not merge until an incomplete full crawl leaves the import timestamp unchanged so scheduled delta imports can still recover missed titles. A completeness shortfall currently logs only a warning and then advances the timestamp used by future delta imports, allowing unchanged titles missed by the crawl to remain absent indefinitely. Files Needing Attention: src/palace/manager/celery/tasks/overdrive.py
|
| Filename | Overview |
|---|---|
| src/palace/manager/celery/tasks/overdrive.py | Adds cursor-driven full-import orchestration and fault handling, but advances the delta-import timestamp despite a completeness shortfall. |
| src/palace/manager/integration/license/overdrive/importer.py | Adds product-list page hydration and application while preserving parent/child metadata behavior. |
| src/palace/manager/integration/license/overdrive/api.py | Extracts reusable asynchronous product hydration without changing update-feed hydration behavior. |
| src/palace/manager/integration/license/overdrive/crawl.py | Makes crawl-fault wording consumer-neutral while preserving cursor arithmetic and completeness checks. |
Sequence Diagram
sequenceDiagram
participant Task as Overdrive import task
participant Cursor as CrawlCursor
participant API as Overdrive API
participant Importer as OverdriveImporter
participant Apply as Apply queue
participant Timestamp as Import timestamp
Task->>Cursor: Restore or create cursor
Task->>Importer: import_products_page(cursor)
Importer->>API: Fetch product-list page
API-->>Importer: Raw products and page totals
Importer->>API: Hydrate metadata and availability
API-->>Importer: Hydrated products
Importer->>Apply: Queue bibliographic/circulation updates
Importer->>Cursor: advance(page)
alt More pages
Cursor-->>Task: Next CrawlCursor
Task->>Task: replace(cursor)
else Structurally complete
Cursor-->>Task: CrawlComplete
Task->>Task: Check distinct-title completeness
Task->>Timestamp: Mark import finished
else Structural fault
Cursor-->>Task: CrawlFault
Task-->>Task: Return partial identifier set without timestamp update
end
Reviews (1): Last reviewed commit: "Walk the full import over the product li..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| timestamp = importer.get_timestamp() | ||
| timestamp.start = start_time | ||
| timestamp.finish = utc_now() |
There was a problem hiding this comment.
When a completed crawl reports a distinct-title shortfall beyond its churn allowance, this branch still advances the import timestamp. Subsequent delta imports start at that new timestamp, causing unchanged titles missed by the full crawl to remain absent indefinitely because no scheduled full import revisits them.
Knowledge Base Used: Celery Tasks and Background Jobs
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## bugfix/overdrive-reap-set-difference #3641 +/- ##
========================================================================
- Coverage 93.56% 93.53% -0.04%
========================================================================
Files 510 510
Lines 47005 46891 -114
Branches 6422 6408 -14
========================================================================
- Hits 43980 43858 -122
- Misses 1955 1960 +5
- Partials 1070 1073 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1 participant
Description
Moves the Overdrive full import (
import_all=True) off the update feed and onto the same product-list crawl the reaper walks. Stacked on #3628, which built the crawl; this PR makes it serve both consumers that need every title.fetch_book_info_listfused two jobs: walking the update feed's pages, and fetching each title's metadata and availability documents. The per-title half moves tohydrate_products, which takes raw product dictionaries from any source and attaches the documents;fetch_book_info_listdelegates to it, unchanged in behavior. This is also the single place a later switch to Overdrive's bulk metadata and bulk availability endpoints will land.lastUpdateTimepinned to the epoch — an ordering that reshuffles on every circulation event anywhere in the collection, so a multi-hour walk over it skips titles exactly the way the reaper's forward walk did, andtotalItemsdecrements in step so no count check can see it. Theimport_allpath now drives the sameCrawlCursorover thedateAdded:ascproduct list — backwards, overlapping pages, ending on a fresh page at offset 0 — with the importer hydrating each page's raw products (OverdriveImporter.import_products_page). The parent/child metadata optimization carries over unchanged: an Advantage collection's crawl still fetches metadata lazily, skipping titles the parent already imported.The delta import is untouched — the update feed is the right primitive for harvesting changes; what it is wrong for is enumerating everything. Per-title hydration still costs a metadata and an availability request per title; cutting that ~50× with the bulk endpoints is the next PR in the stack. A full-import page queued by the previous release carries its feed url and is finished the way it was started, so in-flight runs survive a deploy; the crawl takes over runs that begin afterwards.
Motivation and Context
Follow-on to #3628, second step of unifying Overdrive's harvest paths onto one crawl machinery. The skip mechanism this fixes is the one that PR measured live: removing (or re-updating) a title mid-walk shifts everything behind it down one position under offset paging, and the update-feed ordering churns constantly — the worst possible ordering to page a multi-hour backfill over. The product list's
dateAddedordering is immutable and its ties are stable, which is why the reaper's cursor walks it; a full import needs exactly the same coverage guarantees, just with a different response to faults.How Has This Been Tested?
import_products_pageat the importer level — hydration flags for main vs Advantage collections, cursor hand-back, identifier-set population, and lazy metadata for titles in the parent set; the task-level crawl path — a fresh cursor onimport_all, the cursor round-tripping throughtask.replace(), a fault keeping the partial result while leaving the timestamp untouched, the completeness gate warning without failing the run, and a legacy feed-url page being honored on the old path; andhydrate_productshydrating raw product dictionaries in place. The end-to-end group test now drives a full import from raw HTTP fixtures (product page over the sync client, metadata and availability over the async client) through the apply queue to database records.tox -e py312-dockerovertests/manager/celery,tests/manager/integration/license/opds,tests/manager/integration/license/overdrive, andtests/manager/scripts/test_overdrive.py— 886 passing.mypyclean across 1,160 source files.Checklist