diff --git a/.env.scrapper b/.env.scrapper index f4c44ac..c539039 100644 --- a/.env.scrapper +++ b/.env.scrapper @@ -1,4 +1,4 @@ RELEASE=dev VERSION=1 BUILD=3 -FIX=9 +FIX=10 diff --git a/DSL/Resql/ckb/GET/source_file/get_source_file_exists_by_url.sql b/DSL/Resql/ckb/GET/source_file/get_source_file_exists_by_url.sql index e2a2485..db9cb25 100644 --- a/DSL/Resql/ckb/GET/source_file/get_source_file_exists_by_url.sql +++ b/DSL/Resql/ckb/GET/source_file/get_source_file_exists_by_url.sql @@ -1,7 +1,7 @@ /* declaration: version: 0.1 - description: "Check if a non-deleted source_file already exists for a source by URL" + description: "Check if a source_file already exists (or has ever existed, including deleted) for a source by URL -- used by sitemap discovery to decide whether a URL is genuinely new. A deleted file must still count as \"exists\" here, otherwise a page a user explicitly removed gets silently re-created as a brand-new record the next time the source is discovered/refreshed." method: get namespace: source_file returns: json @@ -17,7 +17,7 @@ declaration: fields: - field: exists type: boolean - description: "Whether a matching source_file exists" + description: "Whether a matching source_file exists (deleted or not)" */ SELECT count(*) > 0 AS exists FROM data_collection.source_file @@ -27,5 +27,4 @@ WHERE (base_id, updated_at) IN ( WHERE source_base_id = :source_base_id::UUID GROUP BY base_id ) AND source_base_id = :source_base_id::UUID - AND is_deleted = FALSE AND url = :url; diff --git a/scrapper/api/app.py b/scrapper/api/app.py index 608b5ae..20ccbfe 100644 --- a/scrapper/api/app.py +++ b/scrapper/api/app.py @@ -1,6 +1,6 @@ import json import os -from datetime import datetime +from datetime import UTC, datetime from pathlib import Path from scrapper import settings @@ -79,7 +79,7 @@ def trigger_specified_api_files_scrapper_task( def generate_edited_metadata(task: EditedMetadataTask) -> str: response = requests.get(task.download_url) metadata = response.json() - metadata["edited_at"] = str(datetime.now()) + metadata["edited_at"] = str(datetime.now(UTC)) metadata["metadata"]["edited"] = True path = Path(task.source_file_path) diff --git a/scrapper/scrapper/items.py b/scrapper/scrapper/items.py index d2df80d..4c8dabb 100644 --- a/scrapper/scrapper/items.py +++ b/scrapper/scrapper/items.py @@ -1,5 +1,5 @@ from dataclasses import dataclass, field -from datetime import datetime +from datetime import UTC, datetime from typing import Optional @@ -24,7 +24,13 @@ class MetadataItem: page_title: str external_id: Optional[str] = "" version: str = "1.0" - created_at: str = field(default_factory=lambda: str(datetime.now())) + # Must be UTC, not naive local time -- this value is sent to Postgres as + # `last_scraped_at` (TIMESTAMP WITH TIME ZONE). A naive local timestamp + # gets misinterpreted as already-UTC, silently shifting it by the host's + # UTC offset -- which can permanently satisfy claim queries like + # `last_scraped_at < :reference_time` (itself always computed in UTC), + # causing a file to be re-claimed for scraping indefinitely. + created_at: str = field(default_factory=lambda: str(datetime.now(UTC))) edited_at: str | None = None language: Optional[str] = None diff --git a/scrapper/scrapper/spiders/entire_source_spider.py b/scrapper/scrapper/spiders/entire_source_spider.py index bc9d14c..54cf018 100644 --- a/scrapper/scrapper/spiders/entire_source_spider.py +++ b/scrapper/scrapper/spiders/entire_source_spider.py @@ -59,6 +59,14 @@ def start_url_impl(self) -> Iterator[str]: def urls_iter_impl(self) -> Iterator[str]: scrapped_before = datetime.datetime.now(datetime.UTC).isoformat() + # get-one-source-file-to-scrape claims any file currently `finished` + # and last scraped before `scrapped_before`. This guard is a + # defense-in-depth backstop against re-processing the same file + # twice within one run (e.g. if the claim query's timing window + # ever overlaps) -- it does not address the root cause of a file + # being reported as eligible again, which was a naive (non-UTC) + # timestamp bug in item construction, fixed separately in items.py. + already_claimed_ids: set[str] = set() while True: result = requests.get( @@ -73,6 +81,10 @@ def urls_iter_impl(self) -> Iterator[str]: link = LinkToScrape(**result.json()["response"][0]) + if link.id in already_claimed_ids: + return + already_claimed_ids.add(link.id) + self.urls.append(link) yield link.url.unicode_string() diff --git a/scrapper/scrapper/spiders/sitemap_collect_spider.py b/scrapper/scrapper/spiders/sitemap_collect_spider.py index ce7470f..fdffcb1 100644 --- a/scrapper/scrapper/spiders/sitemap_collect_spider.py +++ b/scrapper/scrapper/spiders/sitemap_collect_spider.py @@ -123,6 +123,10 @@ async def parse( # pages' content is entire_source_spider's job (it does the # hash comparison against the stored version); creating a # source_file for it again here would duplicate it. + # This check also counts explicitly deleted files as "existing" + # (see get_source_file_exists_by_url.sql), so a URL a user + # removed on purpose stays excluded on future refreshes instead + # of silently reappearing as a new record. already_exists = requests.get( f"{self.settings.get('RUUTER_INTERNAL')}/ckb/source-file/get-source-file-exists-by-url", params={"source_id": self.task.source_id, "url": response.url},