Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions diff_runs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 "
Expand Down
11 changes: 11 additions & 0 deletions output_writer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -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,
Expand Down
17 changes: 17 additions & 0 deletions page_flow.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
13 changes: 12 additions & 1 deletion playwright_scraper.py
Original file line number Diff line number Diff line change
Expand Up @@ -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).",
Expand Down Expand Up @@ -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",
Expand Down
13 changes: 12 additions & 1 deletion puppeteer_scraper.py
Original file line number Diff line number Diff line change
Expand Up @@ -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).",
Expand Down Expand Up @@ -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",
Expand Down
13 changes: 12 additions & 1 deletion selenium_scraper.py
Original file line number Diff line number Diff line change
Expand Up @@ -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).",
Expand Down Expand Up @@ -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",
Expand Down
35 changes: 32 additions & 3 deletions smoke_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down
Loading