style(scrapy): tighten comments and docstrings

Author: vdusek

src/apify/scrapy/_serialization.py

@@ -6,30 +6,21 @@
 
 from pydantic import BaseModel
 
-# Scrapy persists requests and cached responses by serializing the dict produced by
-# `Request.to_dict()` (and a small response payload for the HTTP cache). The Apify integration
-# stores that payload inside the request queue and the key-value store and reads it back later.
-# Those storages hold JSON, so the payload is serialized as JSON here rather than as a pickled
-# Python object graph: JSON is a plain, portable, interoperable data format and carries no
-# executable object state.
+# Scrapy requests and cached responses are stored in the Apify request queue and key-value store,
+# which hold JSON, so they are serialized as JSON here rather than pickled.
 #
-# JSON cannot represent everything Scrapy emits. Of the values that actually appear, only two are
-# not natively JSON-serializable, and both sit at fixed, known keys with a known type:
-#   - `body`: `bytes`
-#   - `headers`: a `{bytes: [bytes]}` mapping (header name -> list of values)
-# These are base64-encoded in place. Pydantic models (e.g. Crawlee's `UserData`, which the Apify
-# integration injects into `meta['userData']`) are converted via `model_dump()`. Everything else —
-# notably the user-controlled `meta` and `cb_kwargs` — must already be JSON-serializable; if it is
-# not, serialization fails with a clear error rather than silently dropping the request. No in-band
-# sentinel is used for user data, so no legitimate value can collide with the encoding scheme.
+# Only `body` (`bytes`) and `headers` (`{bytes: [bytes]}`) are not natively JSON-serializable; both
+# sit at fixed keys and are base64-encoded in place. Pydantic models such as Crawlee's `UserData`
+# are dumped via `model_dump()`. Everything else, notably `meta` and `cb_kwargs`, must already be
+# JSON-serializable, otherwise serialization fails with a clear error. No in-band sentinel is used,
+# so no user value can collide with the encoding.
 
 
 def encode_to_json(data: dict[str, Any]) -> str:
     """Serialize a Scrapy request/response dict to a JSON string.
 
-    The binary `body` and `headers` fields are base64-encoded in place. All other fields must be
-    JSON-serializable; pydantic models are dumped to plain dicts. A clear `TypeError` is raised if
-    any remaining value (typically something in `meta` or `cb_kwargs`) cannot be JSON-encoded.
+    The binary `body` and `headers` fields are base64-encoded in place; pydantic models are dumped
+    to plain dicts. A `TypeError` is raised if any other value cannot be JSON-encoded.
 
     Args:
         data: The dict to serialize, e.g. the output of `scrapy.Request.to_dict()`.
@@ -83,11 +74,7 @@ def decode_from_json(text: str) -> Any:
 
 
 def _json_default(obj: Any) -> Any:
-    """Fallback for values `json.dumps` cannot serialize on its own.
-
-    Only pydantic models are accepted (and dumped to plain dicts); anything else raises, which
-    `encode_to_json` turns into an actionable error.
-    """
+    """Fallback for values `json.dumps` cannot serialize: pydantic models are dumped, anything else raises."""
     if isinstance(obj, BaseModel):
         return obj.model_dump(by_alias=True)
     raise TypeError(f'Object of type {type(obj).__name__} is not JSON-serializable')

src/apify/scrapy/extensions/_httpcache.py

@@ -36,9 +36,8 @@ class ApifyCacheStorage:
     """
 
     def __init__(self, settings: BaseSettings) -> None:
-        # Upper bound on how many keys the per-spider-close cleanup sweeps. Configurable because the
-        # cleanup is bounded and best-effort (see `close_spider`); the default keeps the historical
-        # behavior.
+        # Upper bound on how many keys the per-spider-close cleanup sweeps (best-effort; see
+        # `close_spider`).
         self._expiration_max_items: int = settings.getint('APIFY_HTTPCACHE_EXPIRATION_MAX_ITEMS', 100)
         self._expiration_secs: int = settings.getint('HTTPCACHE_EXPIRATION_SECS')
         self._spider: Spider | None = None
@@ -82,10 +81,9 @@ def close_spider(self, _: Spider, current_time: int | None = None) -> None:
             async def expire_kvs() -> None:
                 if self._kvs is None:
                     raise ValueError('Key value store not initialized')
-                # Best-effort, bounded cleanup: at most `_expiration_max_items` keys are swept per
-                # spider close and `iterate_keys()` order is not guaranteed, so stale entries may
-                # linger across runs. This only reclaims storage; correctness is handled at read
-                # time, where `retrieve_response` treats an expired entry as a cache miss.
+                # Best-effort cleanup: at most `_expiration_max_items` keys per close, in no
+                # guaranteed order, so stale entries may linger. This only reclaims storage;
+                # `retrieve_response` already treats an expired entry as a cache miss.
                 processed = 0
                 async for item in self._kvs.iterate_keys():
                     if processed >= self._expiration_max_items:
@@ -135,8 +133,8 @@ def retrieve_response(self, _: Spider, request: Request, current_time: int | Non
         if current_time is None:
             current_time = int(time())
 
-        # A malformed or legacy (e.g. pickle-format) cache entry must not crash retrieval. Treat it
-        # as a cache miss so Scrapy re-fetches and re-stores it in the current format.
+        # A malformed or legacy cache entry must not crash retrieval; treat it as a cache miss so
+        # Scrapy re-fetches and re-stores it in the current format.
         try:
             if 0 < self._expiration_secs < current_time - read_gzip_time(value):
                 logger.debug('Cache expired', extra={'request': request})
@@ -176,11 +174,10 @@ def store_response(self, _: Spider, request: Request, response: Response) -> Non
 
 
 def to_gzip(data: dict, mtime: int | None = None) -> bytes:
-    """Dump a dictionary to a gzip-compressed byte stream as JSON.
+    """Dump a dictionary to a gzip-compressed JSON byte stream.
 
-    Cached entries are stored in and read back from the Apify key-value store, which holds JSON, so
-    the payload is serialized as JSON rather than pickled. See `apify.scrapy._serialization` for the
-    encoding details.
+    Cache entries live in the Apify key-value store, which holds JSON, so they are serialized as
+    JSON rather than pickled. See `apify.scrapy._serialization` for the encoding.
     """
     payload = encode_to_json(data).encode('utf-8')
     with io.BytesIO() as byte_stream:

src/apify/scrapy/requests.py

@@ -22,13 +22,10 @@
 def _ensure_known_request_class(request_dict: dict[str, Any]) -> None:
     """Validate the optional `_class` entry before `request_from_dict` resolves it.
 
-    `scrapy.utils.request.request_from_dict` resolves a `_class` entry via `load_object`, which
-    imports the dotted path it is given. To keep reconstruction self-contained — importing nothing
-    that the running spider has not already imported — we only accept a `_class` that is already
-    present in `sys.modules` and is a `scrapy.Request` subclass.
-
-    A spider that reads its own requests always has its request classes imported by the time the
-    requests are reconstructed, so this does not restrict legitimate use.
+    `request_from_dict` imports the `_class` dotted path via `load_object`. To avoid importing
+    anything the running spider has not already imported, only a `_class` already present in
+    `sys.modules` and subclassing `scrapy.Request` is accepted. A spider reading its own requests
+    always has those classes imported by then, so legitimate use is unaffected.
     """
     class_path = request_dict.get('_class')
     if class_path is None:
@@ -66,12 +63,8 @@ def to_apify_request(scrapy_request: ScrapyRequest, spider: Spider) -> ApifyRequ
 
     # Configuration to behave as similarly as possible to Scrapy's default RFPDupeFilter.
     #
-    # `payload` carries the request body, which is used both for platform processing and for
-    # computing the extended unique key. The body is also part of the serialized Scrapy request
-    # stored further below, where it is needed to faithfully reconstruct the request. Both copies
-    # originate from `scrapy_request.body` and are kept intentionally: dropping `payload` would
-    # change deduplication, and dropping the serialized copy would couple reconstruction to the
-    # Apify payload.
+    # The body is stored twice on purpose: as `payload` (used for the extended unique key) and inside
+    # the serialized Scrapy request below (used to reconstruct it). Both come from `scrapy_request.body`.
     request_kwargs: dict[str, Any] = {
         'url': scrapy_request.url,
         'method': scrapy_request.method,
@@ -104,11 +97,9 @@ def to_apify_request(scrapy_request: ScrapyRequest, spider: Spider) -> ApifyRequ
 
         request_kwargs['user_data'] = user_data if isinstance(user_data, dict) else {}
 
-        # Convert Scrapy's headers to HttpHeaders and store them on the apify_request. This is only
-        # the Apify-platform-level view of the headers; the authoritative copy, with exact bytes,
-        # travels inside the serialized scrapy_request below. `to_unicode_dict()` decodes as UTF-8
-        # and raises on non-UTF-8 header values, so it is guarded: a request with binary headers
-        # keeps them in the serialized payload instead of being dropped entirely.
+        # Store an Apify-platform view of the headers. The authoritative copy with exact bytes
+        # travels in the serialized scrapy_request below, so non-UTF-8 headers (which make
+        # `to_unicode_dict()` raise) are tolerated rather than dropping the whole request.
         if isinstance(scrapy_request.headers, Headers):
             try:
                 headers = cast('dict[str, str]', dict(scrapy_request.headers.to_unicode_dict()))
@@ -130,12 +121,9 @@ def to_apify_request(scrapy_request: ScrapyRequest, spider: Spider) -> ApifyRequ
         logger.warning(f'Conversion of Scrapy request {scrapy_request} to Apify request failed; {exc}')
         return None
 
-    # Serialize the Scrapy request and store it (base64-encoded JSON) under 'scrapy_request' in the
-    # Apify request's user data. This is intentionally outside the broad except above so that a
-    # non-JSON-serializable value in `meta`/`cb_kwargs` is reported loudly rather than hidden as a
-    # generic warning. The failure is logged with a full traceback and the request is skipped (None
-    # is returned, honoring this function's contract) instead of crashing the whole crawl. See
-    # `_serialization` for the encoding details.
+    # Serialize the Scrapy request as base64-encoded JSON under 'scrapy_request'. Kept outside the
+    # broad except above so a non-JSON-serializable `meta`/`cb_kwargs` is logged with a traceback and
+    # the request skipped (returning None per this function's contract), rather than crashing the crawl.
     try:
         scrapy_request_json = encode_to_json(scrapy_request_dict)
     except TypeError: