kuhyx/testsAndMisc
1
0
Fork 0
mirror of https://github.com/kuhyx/testsAndMisc.git synced 2026-11-11 19:14:02 +01:00
Code Issues Actions 6 Packages Projects Releases Wiki Activity

Split wsg_grabber/downloader.py under the 250-line cap

Browse Source 
Worker did two separable jobs. Both become mixins, because the tests drive
their methods as bound attributes (worker._download_one(), worker._fetch_thread)
and free functions would have broken that:

  _scan.py    ScanMixin      -- threads.json, the archive, one changed thread
  _fetch.py   DownloadMixin  -- claim a file, fetch it, record the outcome

_worker_base.py holds WorkerBase(ABC) with the state both halves touch and the
three methods Worker supplies. Two earlier shapes were abandoned rather than
suppressed: NotImplementedError stubs are uncovered lines under the 100% gate,
and `if TYPE_CHECKING` stubs satisfy mypy but leave pylint reporting no-member.
The ABC satisfies both with no disable comment anywhere.

_downloaded needs a real class-level default: _fetch does `self._downloaded += 1`
and pylint only accepts an attribute as assignable when there is something to
assign to. Worker.__init__ still owns the instance value.

downloader.py 420 -> 238, _scan.py 134, _fetch.py 98, _worker_base.py 52.
277 tests pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011w2Pj4Ek3ZNMQVYcFHgXDY
This commit is contained in:
Krzysztof Rudnicki 2026-08-16 16:01:11 +02:00
parent f58caea2ab
commit 324ec0c9b5
6 changed files with 355 additions and 193 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
docs/superpowers/contracts/wsg-grabber-downloader-split-2026-08-16.json Normal file
View File

@ -0,0 +1,17 @@
{
"title": "Split wsg_grabber/downloader.py under the 250-line cap",
"objective": "Bring python_pkg/wsg_grabber/downloader.py (420 lines) under the repo-wide 250-line file cap by separating the Worker class's two independent halves -- walking the board and fetching a claimed file -- into mixins, with no change to behaviour, to the public Worker API, or to how the tests drive it.",
"acceptance_criteria": [
"downloader.py, _scan.py, _fetch.py and _worker_base.py are each at most 250 lines.",
"All 277 python_pkg/wsg_grabber tests pass unchanged, including the ones that call worker._download_one() and worker._fetch_thread() as bound methods.",
"The package holds 100% branch coverage: no new uncovered lines are introduced by the split (in particular, no NotImplementedError stub bodies).",
"pre-commit passes on all four files with no suppression added -- no # noqa, no # type: ignore, no pylint disable, no new per-file-ignores in meta/pyproject.toml."
],
"out_of_scope": [
"Any behaviour, retry, backoff or state-transition change.",
"Any change to the test files.",
"Splitting the wsg_grabber test files -- that is separate work.",
"Adding pylint mixin configuration to meta/pyproject.toml."
],
"verifier": "pre-commit run --files python_pkg/wsg_grabber/downloader.py python_pkg/wsg_grabber/_scan.py python_pkg/wsg_grabber/_fetch.py python_pkg/wsg_grabber/_worker_base.py (runs ruff, ruff format, mypy, pylint, bandit and the coverage-gated pytest), plus bash ~/utils/scripts/check_file_length.sh --all for the cap itself."
}

43
docs/superpowers/evidence/wsg-grabber-downloader-split-2026-08-16.json Normal file
View File

@ -0,0 +1,43 @@
{
"intent": "Bring python_pkg/wsg_grabber/downloader.py under the 250-line file cap by splitting the Worker class's board-walking and file-fetching halves into mixins, keeping every method reachable as a bound attribute of Worker.",
"scope": [
"python_pkg/wsg_grabber/downloader.py",
"python_pkg/wsg_grabber/_scan.py (new)",
"python_pkg/wsg_grabber/_fetch.py (new)",
"python_pkg/wsg_grabber/_worker_base.py (new)",
"Non-goal: no behaviour change, no test changes, no pylint config change"
],
"changes": [
"Moved the board-walking methods -- _scan_step, _refresh_thread_list, _archive_refs, _conditional_get, _fetch_thread, _write_off_deleted -- into ScanMixin in _scan.py.",
"Moved _download_one and _apply_result into DownloadMixin in _fetch.py.",
"Added _worker_base.py holding WorkerBase(ABC): the state both halves touch, plus abstract _get_json / _publish / _wait_for_slot which Worker supplies. Both mixins inherit from it, so neither redeclares the other's bookkeeping.",
"Worker is now `class Worker(ScanMixin, DownloadMixin)`; __init__ is unchanged and remains the only place instance state is created.",
"downloader.py 420 -> 238 lines; _scan.py 134; _fetch.py 98; _worker_base.py 52."
],
"verification": [
{
"command": "python3 -m pytest python_pkg/wsg_grabber/tests/ -q",
"result": "pass",
"evidence": "277 passed in 15.20s, unchanged from before the split. This is the load-bearing check: test_downloader_part2.py calls worker._download_one() and worker._fetch_thread(ThreadRef(7, 100)) as bound methods and sets worker._catalog_fresh directly, so the mixin composition is exercised rather than assumed."
},
{
"command": "pre-commit run --files python_pkg/wsg_grabber/downloader.py python_pkg/wsg_grabber/_scan.py python_pkg/wsg_grabber/_fetch.py python_pkg/wsg_grabber/_worker_base.py",
"result": "pass",
"evidence": "ruff, ruff format, mypy, pylint, bandit and the coverage-gated pytest all pass. The only remaining hook failures were 'no staged evidence artifact' and 'no workflow contract', which this file and the paired contract satisfy. No suppression was added."
},
{
"command": "bash ~/utils/scripts/check_file_length.sh --all | grep -c 'over by'",
"result": "pass",
"evidence": "All four files are under the cap: 238 / 134 / 98 / 52."
}
],
"risks": [
"Two intermediate designs failed the gate and were abandoned rather than suppressed. Bare `raise NotImplementedError` stubs on the mixins would have been uncovered lines under the 100% gate; `if TYPE_CHECKING` method stubs satisfied mypy but left pylint reporting no-member. The ABC with real abstract methods satisfies both.",
"_downloaded carries a class-level default of 0 rather than a bare annotation, because _fetch does `self._downloaded += 1` and pylint only accepts an attribute as assignable when there is something to assign to. Worker.__init__ still sets the instance value, so no state is shared between instances -- ints are immutable, so the class attribute is never mutated in place.",
"Mixin method resolution order now matters: Worker(ScanMixin, DownloadMixin) means ScanMixin wins on any name collision. There are none today; adding a same-named method to both would silently pick ScanMixin's."
],
"rollback": [
"git revert the commit; downloader.py returns to its 420-line form and the three new modules are deleted.",
"After rollback, run python3 -m pytest python_pkg/wsg_grabber/tests/ -q to confirm 277 tests still pass."
]
}

98
python_pkg/wsg_grabber/_fetch.py Normal file
View File

@ -0,0 +1,98 @@
"""Fetching one claimed file and recording how it went.
Split out of :mod:`python_pkg.wsg_grabber.downloader` to keep it under the
250-line cap. A mixin rather than free functions because the tests drive
``worker._download_one()`` as a bound method and it shares the worker's
download tally.
"""
from __future__ import annotations
from typing import TYPE_CHECKING
from python_pkg.wsg_grabber import catalog, net, scanner, store
from python_pkg.wsg_grabber._worker_base import WorkerBase
from python_pkg.wsg_grabber.models import Downloaded, FileReady, Outcome
from python_pkg.wsg_grabber.states import FileEvent, FileState, next_state
if TYPE_CHECKING:
from pathlib import Path
from python_pkg.wsg_grabber.models import RemoteFile
class DownloadMixin(WorkerBase):
"""The file-fetching half of :class:`~.downloader.Worker`."""
def _download_one(self) -> None:
"""Claim the next pending file and fetch it."""
claimed = store.claim_next(self._deps.conn)
if claimed is None:
return
part = (
self._deps.incoming
/ f"{store.local_name(claimed.md5, claimed.tim, claimed.ext)}.part"
)
self._wait_for_slot()
result = net.download(
self._deps.session,
net.Transfer(
url=catalog.media_url(claimed.tim, claimed.ext),
part_path=part,
expected_md5=claimed.md5,
expected_size=claimed.fsize,
should_stop=self._deps.stop.is_set,
on_progress=lambda done: store.record_progress(
self._deps.conn,
claimed.md5,
done,
),
),
)
self._apply_result(claimed, part, result.outcome, result.retry_after)
def _apply_result(
self,
claimed: RemoteFile,
part: Path,
outcome: Outcome,
retry_after: float | None,
) -> None:
"""Translate a download outcome into index state and an event.
Args:
claimed: The file that was attempted.
part: Its partial-file path.
outcome: How the download ended.
retry_after: Server-requested pause, when supplied.
"""
if outcome is Outcome.ABORTED:
store.set_state(self._deps.conn, claimed.md5, FileState.FAILED)
return
if outcome is Outcome.COMPLETED:
final = part.with_suffix("")
part.replace(final)
store.mark_downloaded(self._deps.conn, claimed.md5, final.name)
self._downloaded += 1
self._publish(
FileReady(
item=store.ready_item(claimed, final),
),
)
self._publish(Downloaded(count=self._downloaded))
return
event = {
Outcome.NOT_FOUND: FileEvent.NOT_FOUND,
Outcome.CHECKSUM_MISMATCH: FileEvent.CHECKSUM_MISMATCH,
Outcome.TRANSIENT: FileEvent.TRANSIENT_ERROR,
}[outcome]
attempts = store.attempts_for(self._deps.conn, claimed.md5)
store.set_state(
self._deps.conn,
claimed.md5,
next_state(FileState.DOWNLOADING, event, attempts),
error=outcome.value,
)
if retry_after is not None:
self._deps.stop.wait(scanner.backoff_seconds(attempts, retry_after))

134
python_pkg/wsg_grabber/_scan.py Normal file
View File

@ -0,0 +1,134 @@
"""Walking the board: the thread list, the archive, and each changed thread.
Split out of :mod:`python_pkg.wsg_grabber.downloader` to keep it under the
250-line cap. This is a mixin rather than a set of free functions because the
tests drive these as bound methods (``worker._fetch_thread(...)``), and because
they share the worker's scan bookkeeping.
The bare annotations below declare what ``Worker.__init__`` actually sets, so
mypy can see the attributes without the mixin duplicating any of that state.
"""
from __future__ import annotations
from typing import TYPE_CHECKING
from python_pkg.wsg_grabber import catalog, scanner, store, store_threads
from python_pkg.wsg_grabber._worker_base import WorkerBase
from python_pkg.wsg_grabber.constants import (
API_HOST,
ARCHIVE_URL,
BOARD,
THREADS_URL,
)
from python_pkg.wsg_grabber.models import Indexed, ThreadRef
from python_pkg.wsg_grabber.states import CLAIMABLE, FileState
if TYPE_CHECKING:
from python_pkg.wsg_grabber.models import JsonResponse
class ScanMixin(WorkerBase):
"""The board-walking half of :class:`~.downloader.Worker`."""
def _scan_step(self) -> None:
"""Read the thread list, or fetch the next thread that changed."""
if not self._catalog_fresh:
self._refresh_thread_list()
return
ref = self._pending_threads.pop(0)
self._fetch_thread(ref)
def _refresh_thread_list(self) -> None:
"""Fetch ``threads.json`` and work out which threads to visit."""
response = self._conditional_get(THREADS_URL)
live = catalog.parse_thread_list(response.payload)
if self._deps.include_archive:
live = live + self._archive_refs()
known = store_threads.known_threads(self._deps.conn)
self._pending_threads = scanner.stale_threads(known, live)
self._catalog_fresh = True
self._finished_announced = False
def _archive_refs(self) -> list[ThreadRef]:
"""Fetch the archive listing, tolerating boards without one.
Returns:
list[ThreadRef]: Archived threads, empty when unavailable.
"""
response = self._conditional_get(ARCHIVE_URL)
if response.not_found:
return []
return catalog.parse_archive(response.payload)
def _conditional_get(self, url: str) -> JsonResponse:
"""Fetch a board-level endpoint, sending back the stamp we hold.
Without this an idle reviewer re-fetches threads.json and archive.json
unconditionally every cycle. With it an unchanged board costs a 304.
Args:
url: Absolute API URL.
Returns:
JsonResponse: The response; on 304 the payload is None and the
caller keeps whatever it already knew.
"""
stamp = store_threads.cached_last_modified(self._deps.conn, url)
response = self._get_json(url, stamp)
if not response.not_modified:
store_threads.record_last_modified(
self._deps.conn,
url,
response.last_modified,
)
return response
def _fetch_thread(self, ref: ThreadRef) -> None:
"""Read one thread and record any files it introduces.
Args:
ref: Thread to fetch.
"""
url = f"{API_HOST}/{BOARD}/thread/{ref.thread_no}.json"
stamp = store_threads.http_last_modified(self._deps.conn, ref.thread_no)
response = self._get_json(url, stamp)
if response.not_found:
store_threads.mark_thread_gone(self._deps.conn, ref.thread_no)
return
if response.not_modified:
store_threads.record_thread(self._deps.conn, ref, stamp)
return
parsed = catalog.parse_thread(ref.thread_no, response.payload)
known = store.known_md5s(self._deps.conn)
added = store.record_files(
self._deps.conn,
catalog.new_files(parsed, known),
)
self._write_off_deleted(catalog.deleted_md5s(response.payload), known)
store_threads.record_thread(self._deps.conn, ref, response.last_modified)
if added:
self._publish(Indexed(known=len(known) + added))
def _write_off_deleted(self, gone: set[str], known: set[str]) -> None:
"""Mark not-yet-fetched files terminal when the board drops them.
Only files still waiting to download are affected. Once the bytes are
on disk the post being deleted upstream is irrelevant, so an already
downloaded or reviewed file keeps its state -- the same rule the
transition table encodes by rejecting ``READY + DELETED_UPSTREAM``.
Args:
gone: md5s the board reports as deleted.
known: md5s already in the index.
"""
for digest in gone & known:
if store.state_of(self._deps.conn, digest) not in CLAIMABLE:
continue
store.set_state(
self._deps.conn,
digest,
FileState.GONE,
error="deleted upstream",
)

52
python_pkg/wsg_grabber/_worker_base.py Normal file
View File

@ -0,0 +1,52 @@
"""The state and collaborators the worker's two halves both rely on.
:class:`~python_pkg.wsg_grabber.downloader.Worker` is assembled from two mixins
-- :mod:`._scan` and :mod:`._fetch` -- so that no single module exceeds the
250-line cap. Each half touches bookkeeping the other half owns, so both
inherit from here rather than declaring the same attributes twice.
The annotations carry no values: ``Worker.__init__`` remains the only place
this state is created. The two methods are abstract because ``Worker`` supplies
them; declaring them gives mypy and pylint something real to resolve against,
which a bare mixin does not.
"""
from __future__ import annotations
from abc import ABC, abstractmethod
from typing import TYPE_CHECKING
if TYPE_CHECKING:
from python_pkg.wsg_grabber.downloader import WorkerDeps
from python_pkg.wsg_grabber.models import (
DownloadEvent,
JsonResponse,
ThreadRef,
)
class WorkerBase(ABC):
"""State and collaborators shared by the worker's two halves."""
_deps: WorkerDeps
_pending_threads: list[ThreadRef]
_catalog_fresh: bool
_finished_announced: bool
# Given a real class-level default rather than a bare annotation: _fetch
# increments it (`self._downloaded += 1`), and pylint only treats an
# attribute as existing for assignment when there is something to assign
# to. Worker.__init__ still sets the instance value on construction.
_downloaded: int = 0
@abstractmethod
def _get_json(self, url: str, stamp: str | None = None) -> JsonResponse:
"""Fetch a JSON endpoint, pacing requests and counting the last one."""
@abstractmethod
def _publish(self, event: DownloadEvent) -> None:
"""Hand an event to the UI thread."""
@abstractmethod
def _wait_for_slot(self) -> None:
"""Block until the polite inter-request delay has elapsed."""

204
python_pkg/wsg_grabber/downloader.py
View File

@ -21,32 +21,20 @@ from typing import TYPE_CHECKING
import requests
from python_pkg.wsg_grabber import catalog, net, scanner, store, store_threads
from python_pkg.wsg_grabber import net, scanner, store
from python_pkg.wsg_grabber._fetch import DownloadMixin
from python_pkg.wsg_grabber._scan import ScanMixin
from python_pkg.wsg_grabber.constants import (
API_HOST,
ARCHIVE_URL,
BOARD,
IDLE_POLL_S,
IDLE_RESCAN_S,
THREADS_URL,
)
from python_pkg.wsg_grabber.models import (
Downloaded,
FileReady,
Indexed,
JsonResponse,
Outcome,
ScanFinished,
TaskKind,
ThreadRef,
WorkerFailed,
)
from python_pkg.wsg_grabber.states import (
CLAIMABLE,
FileEvent,
FileState,
next_state,
)
_LOGGER = logging.getLogger(__name__)
@ -71,7 +59,7 @@ if TYPE_CHECKING:
from pathlib import Path
import queue
from python_pkg.wsg_grabber.models import DownloadEvent, RemoteFile
from python_pkg.wsg_grabber.models import DownloadEvent
@dataclass
@ -86,8 +74,13 @@ class WorkerDeps:
include_archive: bool = True
class Worker:
"""Walks /wsg/ and downloads every file it has not seen before."""
class Worker(ScanMixin, DownloadMixin):
"""Walks /wsg/ and downloads every file it has not seen before.
The board-walking and file-fetching halves live in _scan and _fetch to
keep this module under the 250-line cap; they are mixins because the
tests drive their methods as bound attributes of a Worker.
"""
def __init__(self, deps: WorkerDeps) -> None:
"""Store collaborators and initialise scan bookkeeping.
@ -167,181 +160,6 @@ class Worker:
self._catalog_fresh = False
self._finished_announced = False
def _scan_step(self) -> None:
"""Read the thread list, or fetch the next thread that changed."""
if not self._catalog_fresh:
self._refresh_thread_list()
return
ref = self._pending_threads.pop(0)
self._fetch_thread(ref)
def _refresh_thread_list(self) -> None:
"""Fetch ``threads.json`` and work out which threads to visit."""
response = self._conditional_get(THREADS_URL)
live = catalog.parse_thread_list(response.payload)
if self._deps.include_archive:
live = live + self._archive_refs()
known = store_threads.known_threads(self._deps.conn)
self._pending_threads = scanner.stale_threads(known, live)
self._catalog_fresh = True
self._finished_announced = False
def _archive_refs(self) -> list[ThreadRef]:
"""Fetch the archive listing, tolerating boards without one.
Returns:
list[ThreadRef]: Archived threads, empty when unavailable.
"""
response = self._conditional_get(ARCHIVE_URL)
if response.not_found:
return []
return catalog.parse_archive(response.payload)
def _conditional_get(self, url: str) -> JsonResponse:
"""Fetch a board-level endpoint, sending back the stamp we hold.
Without this an idle reviewer re-fetches threads.json and archive.json
unconditionally every cycle. With it an unchanged board costs a 304.
Args:
url: Absolute API URL.
Returns:
JsonResponse: The response; on 304 the payload is None and the
caller keeps whatever it already knew.
"""
stamp = store_threads.cached_last_modified(self._deps.conn, url)
response = self._get_json(url, stamp)
if not response.not_modified:
store_threads.record_last_modified(
self._deps.conn,
url,
response.last_modified,
)
return response
def _fetch_thread(self, ref: ThreadRef) -> None:
"""Read one thread and record any files it introduces.
Args:
ref: Thread to fetch.
"""
url = f"{API_HOST}/{BOARD}/thread/{ref.thread_no}.json"
stamp = store_threads.http_last_modified(self._deps.conn, ref.thread_no)
response = self._get_json(url, stamp)
if response.not_found:
store_threads.mark_thread_gone(self._deps.conn, ref.thread_no)
return
if response.not_modified:
store_threads.record_thread(self._deps.conn, ref, stamp)
return
parsed = catalog.parse_thread(ref.thread_no, response.payload)
known = store.known_md5s(self._deps.conn)
added = store.record_files(
self._deps.conn,
catalog.new_files(parsed, known),
)
self._write_off_deleted(catalog.deleted_md5s(response.payload), known)
store_threads.record_thread(self._deps.conn, ref, response.last_modified)
if added:
self._publish(Indexed(known=len(known) + added))
def _write_off_deleted(self, gone: set[str], known: set[str]) -> None:
"""Mark not-yet-fetched files terminal when the board drops them.
Only files still waiting to download are affected. Once the bytes are
on disk the post being deleted upstream is irrelevant, so an already
downloaded or reviewed file keeps its state -- the same rule the
transition table encodes by rejecting ``READY + DELETED_UPSTREAM``.
Args:
gone: md5s the board reports as deleted.
known: md5s already in the index.
"""
for digest in gone & known:
if store.state_of(self._deps.conn, digest) not in CLAIMABLE:
continue
store.set_state(
self._deps.conn,
digest,
FileState.GONE,
error="deleted upstream",
)
def _download_one(self) -> None:
"""Claim the next pending file and fetch it."""
claimed = store.claim_next(self._deps.conn)
if claimed is None:
return
part = (
self._deps.incoming
/ f"{store.local_name(claimed.md5, claimed.tim, claimed.ext)}.part"
)
self._wait_for_slot()
result = net.download(
self._deps.session,
net.Transfer(
url=catalog.media_url(claimed.tim, claimed.ext),
part_path=part,
expected_md5=claimed.md5,
expected_size=claimed.fsize,
should_stop=self._deps.stop.is_set,
on_progress=lambda done: store.record_progress(
self._deps.conn,
claimed.md5,
done,
),
),
)
self._apply_result(claimed, part, result.outcome, result.retry_after)
def _apply_result(
self,
claimed: RemoteFile,
part: Path,
outcome: Outcome,
retry_after: float | None,
) -> None:
"""Translate a download outcome into index state and an event.
Args:
claimed: The file that was attempted.
part: Its partial-file path.
outcome: How the download ended.
retry_after: Server-requested pause, when supplied.
"""
if outcome is Outcome.ABORTED:
store.set_state(self._deps.conn, claimed.md5, FileState.FAILED)
return
if outcome is Outcome.COMPLETED:
final = part.with_suffix("")
part.replace(final)
store.mark_downloaded(self._deps.conn, claimed.md5, final.name)
self._downloaded += 1
self._publish(
FileReady(
item=store.ready_item(claimed, final),
),
)
self._publish(Downloaded(count=self._downloaded))
return
event = {
Outcome.NOT_FOUND: FileEvent.NOT_FOUND,
Outcome.CHECKSUM_MISMATCH: FileEvent.CHECKSUM_MISMATCH,
Outcome.TRANSIENT: FileEvent.TRANSIENT_ERROR,
}[outcome]
attempts = store.attempts_for(self._deps.conn, claimed.md5)
store.set_state(
self._deps.conn,
claimed.md5,
next_state(FileState.DOWNLOADING, event, attempts),
error=outcome.value,
)
if retry_after is not None:
self._deps.stop.wait(scanner.backoff_seconds(attempts, retry_after))
def _announce_finished(self) -> None:
"""Emit ScanFinished once per completed sweep of the board."""
if self._finished_announced: