From 2fef08d2a66d5d29c8f475d3038f2b63ce9c89d7 Mon Sep 17 00:00:00 2001 From: jehrr <79864894+jehrr@users.noreply.github.com> Date: Thu, 8 Oct 2026 13:41:49 +0000 Subject: [PATCH] Add --block-retry-delay (exponential wait on refused pages) and schema_version in the sidecar Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 11 +++++++++++ README.md | 11 +++++++++++ diff_runs.py | 12 ++++++++++++ output_writer.py | 11 +++++++++++ page_flow.py | 17 +++++++++++++++++ playwright_scraper.py | 13 ++++++++++++- puppeteer_scraper.py | 13 ++++++++++++- selenium_scraper.py | 13 ++++++++++++- smoke_test.py | 35 ++++++++++++++++++++++++++++++++--- 9 files changed, 130 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ca93533..844f604 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,17 @@ that is a fix, and the release notes lead with it. ## [Unreleased] +### Added + +- **`--block-retry-delay SECONDS`** (all three engines). Wait before + re-fetching a refused page, doubling per retry. Unset keeps the previous + linear `retry-delay x attempt` wait, so existing runs behave identically. + One `page_flow.block_pause` serves the three engines. The retry count is + unchanged; there is no jitter, since one client has no herd. +- **`schema_version` in the `.meta.json` sidecar** (`1.0.0`), versioning the + output contract rather than the package. `diff_runs.py` refuses two runs + whose MAJOR differs. + ### Fixed - **`--retries 0` sent no request at all and reported exit 3.** Every engine diff --git a/README.md b/README.md index 906df43..a1c1251 100644 --- a/README.md +++ b/README.md @@ -55,6 +55,13 @@ python3 playwright_scraper.py \ That writes `orlando.json`, `orlando.csv` and `orlando.meta.json`. +`orlando.meta.json` carries `schema_version` (`output_writer.SCHEMA_VERSION`, +MAJOR.MINOR.PATCH): MAJOR when a column is removed, renamed or changes +meaning, MINOR when one is added at the end, PATCH for a clarification. +It is the output contract's version, not the package's. `diff_runs.py` +refuses to compare two runs whose MAJORs differ; sidecars written before it +existed carry none and are not refused. + **Install exactly one engine.** The three engines' pins are mutually unsatisfiable (`playwright` and `pyppeteer` disagree on `pyee`; `pyppeteer` and `selenium` on `urllib3`). Use a virtualenv per engine if you need more @@ -237,6 +244,10 @@ expedia_property_id, price_note, sku, url, source. The only differences were Engine flag differences are asserted in both directions by the test suite, so closing one needs a README edit rather than a quiet patch: +* `--block-retry-delay SECONDS` — all three engines, but not part of the + family flag contract. Wait before re-fetching a page that came back + refused, doubling per retry (30 waits 30 s, then 60 s). Unset keeps the + short linear wait. The retry COUNT is unchanged (2 without a pool). * `--browser-channel` — Playwright only * `--chromium-path` — pyppeteer only * `--cdp-connect-timeout` — Playwright and pyppeteer only (Selenium cannot diff --git a/diff_runs.py b/diff_runs.py index a1880d9..5f8d0aa 100644 --- a/diff_runs.py +++ b/diff_runs.py @@ -325,6 +325,18 @@ def _check_comparable(args) -> bool: f"{label} ({path}) was a {status!r} run — stopped after " f"{meta.get('pages_completed')} of {meta.get('pages_requested')} " f"page(s), reason {meta.get('stop_reason')!r}") + majors = {} + for label, path in (("--old", args.old), ("--new", args.new)): + _, meta = _run_status(path) + version = str((meta or {}).get("schema_version") or "") + if version: + majors[label] = version.split(".")[0] + if len(set(majors.values())) > 1: + problems.append( + f"the two runs were written under different output-schema majors " + f"({majors}): a column was removed, renamed or changed meaning " + f"between them, so a field-by-field diff would describe the " + f"schema change rather than the catalogue.") if len(set(modes.values())) > 1: problems.append( f"the two runs are different modes ({modes}). A listing row and a " diff --git a/output_writer.py b/output_writer.py index fa4d9e9..d2fe7f6 100644 --- a/output_writer.py +++ b/output_writer.py @@ -337,6 +337,16 @@ def write_csv(rows: Sequence[Any], path: str, row_cls: Type = Product) -> None: EXIT_FETCH_FAILED = 5 +# The version of the OUTPUT CONTRACT: `Product`'s columns and what they mean, +# plus the sidecar's own fields. Bump MAJOR when a column is removed, renamed +# or changes meaning (a consumer must change code), MINOR when one is added at +# the end (a consumer may ignore it), PATCH for a clarification that changes no +# output. It is deliberately NOT the package version: most releases change no +# output at all, and a consumer should not have to re-validate on those. +# Sidecars written before it existed carry no `schema_version`. +SCHEMA_VERSION = "1.0.0" + + def write_run_meta(out_prefix: str, meta: dict) -> str: """Write a run-metadata sidecar next to the output, return its path. @@ -393,6 +403,7 @@ def run_meta(status: str, stop_reason: str, pages_requested: int, about WHICH part of the catalogue is missing, not just how much. """ meta = { + "schema_version": SCHEMA_VERSION, "source": source, "mode": mode, "status": status, diff --git a/page_flow.py b/page_flow.py index b166574..c01145e 100644 --- a/page_flow.py +++ b/page_flow.py @@ -298,6 +298,23 @@ def is_unpainted(state: str, html: Optional[str]) -> bool: BLOCK_RETRIES_WITHOUT_POOL = 2 +def block_pause(retry_delay: float, block_attempt: int, + backoff_base: Optional[float] = None) -> float: + """Seconds to wait before re-fetching a page that came back refused. + + `block_attempt` counts from 0. Without `--block-retry-delay` this is the + historical linear `retry_delay * (n + 1)` (2 s, 4 s at the defaults), + which is short for a RATE response that was measured clearing minutes + later. With it, the wait starts at `backoff_base` and DOUBLES each retry, + so a reader who wants to outwait the limit asks for 30 and gets 30, 60. + One implementation here so the three engines cannot disagree on it. + There is deliberately no jitter: one client has no herd to spread from. + """ + if backoff_base is None: + return retry_delay * (block_attempt + 1) + return backoff_base * (2 ** block_attempt) + + def attempts_arg(value: str) -> int: """argparse type for `--retries`: an ATTEMPT count, so at least 1. diff --git a/playwright_scraper.py b/playwright_scraper.py index 4ec5025..0ee5e2c 100644 --- a/playwright_scraper.py +++ b/playwright_scraper.py @@ -839,7 +839,8 @@ def _fetch_one_page(session, args, pool, page_num: int, url: Optional[str]) -> P break if block_attempt < block_retries: - pause = args.retry_delay * (block_attempt + 1) + pause = page_flow.block_pause(args.retry_delay, block_attempt, + args.block_retry_delay) if has_pool: logger.warning("Page %d came back as %s from %s — retrying " "from another exit in %.1fs (%d/%d).", @@ -1273,6 +1274,16 @@ def parse_args(): p.add_argument("--retry-delay", type=float, default=2.0, help="Seconds before the first page-load retry, doubling " "thereafter (default 2.0)") + p.add_argument("--block-retry-delay", type=float, default=None, + metavar="SECONDS", + help="Wait before re-fetching a page that came back " + "REFUSED (HTTP 429 / challenge), doubling on each " + "retry: 30 waits 30 s then 60 s. Default: unset, " + "which keeps the short linear wait of --retry-delay " + "x attempt. The 429 here is a rate response that was " + "measured clearing minutes later, so a longer wait " + "is the lever that fits it. The number of retries " + "stays the built-in budget (2 without a proxy pool).") p.add_argument("--format", choices=["json", "csv", "both"], default="both") p.add_argument("--out", default="vrbo_products", help="Output file prefix") p.add_argument("--locale", default="en-US", diff --git a/puppeteer_scraper.py b/puppeteer_scraper.py index 536ea8c..fe9a486 100644 --- a/puppeteer_scraper.py +++ b/puppeteer_scraper.py @@ -590,7 +590,8 @@ def _fetch_one_page(session, args, pool, page_num: int, break if block_attempt < block_retries: - pause = args.retry_delay * (block_attempt + 1) + pause = page_flow.block_pause(args.retry_delay, block_attempt, + args.block_retry_delay) if has_pool: logger.warning("Page %d came back as %s from %s — retrying " "from another exit in %.1fs (%d/%d).", @@ -890,6 +891,16 @@ def parse_args(): p.add_argument("--retry-delay", type=float, default=2.0, help="Seconds before the first page-load retry, doubling " "thereafter (default 2.0)") + p.add_argument("--block-retry-delay", type=float, default=None, + metavar="SECONDS", + help="Wait before re-fetching a page that came back " + "REFUSED (HTTP 429 / challenge), doubling on each " + "retry: 30 waits 30 s then 60 s. Default: unset, " + "which keeps the short linear wait of --retry-delay " + "x attempt. The 429 here is a rate response that was " + "measured clearing minutes later, so a longer wait " + "is the lever that fits it. The number of retries " + "stays the built-in budget (2 without a proxy pool).") p.add_argument("--format", choices=["json", "csv", "both"], default="both") p.add_argument("--out", default="vrbo_products", help="Output file prefix") p.add_argument("--chromium-path", default=None, metavar="PATH", diff --git a/selenium_scraper.py b/selenium_scraper.py index 325a42d..2a31087 100644 --- a/selenium_scraper.py +++ b/selenium_scraper.py @@ -577,7 +577,8 @@ def _fetch_one_page(session, args, pool, page_num: int, break if block_attempt < block_retries: - pause = args.retry_delay * (block_attempt + 1) + pause = page_flow.block_pause(args.retry_delay, block_attempt, + args.block_retry_delay) if has_pool: logger.warning("Page %d came back as %s from %s — retrying " "from another exit in %.1fs (%d/%d).", @@ -879,6 +880,16 @@ def parse_args(): p.add_argument("--retry-delay", type=float, default=2.0, help="Seconds before the first page-load retry, doubling " "thereafter (default 2.0)") + p.add_argument("--block-retry-delay", type=float, default=None, + metavar="SECONDS", + help="Wait before re-fetching a page that came back " + "REFUSED (HTTP 429 / challenge), doubling on each " + "retry: 30 waits 30 s then 60 s. Default: unset, " + "which keeps the short linear wait of --retry-delay " + "x attempt. The 429 here is a rate response that was " + "measured clearing minutes later, so a longer wait " + "is the lever that fits it. The number of retries " + "stays the built-in budget (2 without a proxy pool).") p.add_argument("--format", choices=["json", "csv", "both"], default="both") p.add_argument("--out", default="vrbo_products", help="Output file prefix") p.add_argument("--locale", default="en-US", diff --git a/smoke_test.py b/smoke_test.py index 3f69425..1b96d51 100644 --- a/smoke_test.py +++ b/smoke_test.py @@ -515,6 +515,32 @@ def test_challenge_is_not_always_solvable(): return ok +def test_block_backoff_and_schema_version(): + group("--block-retry-delay backs off; the sidecar carries schema_version") + ok = True + bp = page_flow.block_pause + ok &= check("unset keeps the historical linear wait (2, 4)", + [bp(2.0, n) for n in (0, 1)] == [2.0, 4.0]) + ok &= check("set, it starts at the base and doubles (30, 60, 120)", + [bp(2.0, n, 30.0) for n in (0, 1, 2)] == [30.0, 60.0, 120.0]) + for eng in ("playwright_scraper", "puppeteer_scraper", "selenium_scraper"): + src = open(eng + ".py", encoding="utf-8").read() + ok &= check(f"{eng} takes --block-retry-delay and routes it through block_pause", + '"--block-retry-delay"' in src + and "page_flow.block_pause(args.retry_delay, block_attempt," in src) + import output_writer + meta = output_writer.run_meta("complete", "completed", 1, 1, "u", "u", 1) + ok &= check("run_meta records schema_version", + meta.get("schema_version") == output_writer.SCHEMA_VERSION) + ok &= check("a caller's extra cannot overwrite it", + output_writer.run_meta("complete", "completed", 1, 1, "u", "u", 1, + extra={"schema_version": "9"})["schema_version"] + == output_writer.SCHEMA_VERSION) + ok &= check("it is a MAJOR.MINOR.PATCH string", + re.fullmatch(r"\d+\.\d+\.\d+", output_writer.SCHEMA_VERSION) is not None) + return ok + + def test_retries_zero_is_refused(): group("--retries 0 would never navigate, so the parser refuses it") ok = True @@ -1097,10 +1123,12 @@ def test_engine_parity(skips): # timeout there would be a flag for a path that does not exist. "playwright_scraper": {"--locale", "--fingerprint", "--fp-tags", "--fp-country", "--browser-channel", - "--cdp-connect-timeout"}, + "--cdp-connect-timeout", + "--block-retry-delay"}, "selenium_scraper": {"--locale", "--fingerprint", "--fp-tags", - "--fp-country"}, - "puppeteer_scraper": {"--chromium-path", "--cdp-connect-timeout"}, + "--fp-country", "--block-retry-delay"}, + "puppeteer_scraper": {"--chromium-path", "--cdp-connect-timeout", + "--block-retry-delay"}, } for engine, extra in documented_extra.items(): actual = flagsets[engine] - contract @@ -1581,6 +1609,7 @@ def main() -> int: ok &= test_pagination() ok &= test_page_state() ok &= test_challenge_is_not_always_solvable() + ok &= test_block_backoff_and_schema_version() ok &= test_retries_zero_is_refused() ok &= test_page_flow_policy() ok &= test_scroll_loop()