Merge remote-tracking branch 'origin/master' into media-storages

.pre-commit-config.yaml

@@ -27,7 +27,7 @@ repos:
   hooks:
     - id: sphinx-lint
 - repo: https://github.com/scrapy/sphinx-scrapy
-  rev: 0.8.10
+  rev: 0.8.11
   hooks:
     - id: sphinx-scrapy
 - repo: https://github.com/zizmorcore/zizmor-pre-commit

docs/_ext/scrapydocs.py

@@ -137,14 +137,6 @@ def source_role(
     return [node], []
 
 
-def issue_role(
-    name, rawtext, text: str, lineno, inliner, options=None, content=None
-) -> tuple[list[Any], list[Any]]:
-    ref = "https://github.com/scrapy/scrapy/issues/" + text
-    node = nodes.reference(rawtext, "issue " + text, refuri=ref)
-    return [node], []
-
-
 def commit_role(
     name, rawtext, text: str, lineno, inliner, options=None, content=None
 ) -> tuple[list[Any], list[Any]]:
@@ -164,7 +156,6 @@ def rev_role(
 def setup(app: Sphinx) -> dict[str, Any]:
     app.add_role("source", source_role)
     app.add_role("commit", commit_role)
-    app.add_role("issue", issue_role)
     app.add_role("rev", rev_role)
 
     app.add_node(

docs/faq.rst

@@ -141,7 +141,7 @@ middleware with a :ref:`custom downloader middleware
 
 -   If you can meet the installation requirements, use pyre2_ instead of
     Python’s re_ to compile your URL-filtering regular expression. See
-    :issue:`1908`.
+    :gh:`1908`.
 
 See also `other suggestions at StackOverflow
 <https://stackoverflow.com/q/36440681>`__.
@@ -419,7 +419,7 @@ Running ``runspider`` I get ``error: No spider found in file: <filename>``
 This may happen if your Scrapy project has a spider module with a name that
 conflicts with the name of one of the `Python standard library modules`_, such
 as ``csv.py`` or ``os.py``, or any `Python package`_ that you have installed.
-See :issue:`2680`.
+See :gh:`2680`.
 
 
 .. _has been reported: https://github.com/scrapy/scrapy/issues/2905

docs/requirements.in

@@ -6,4 +6,4 @@ sphinx-notfound-page
 sphinx-reredirects
 sphinx-rtd-theme
 sphinx-rtd-dark-mode
-sphinx-scrapy @ git+https://github.com/scrapy/sphinx-scrapy.git@0.8.10
+sphinx-scrapy @ git+https://github.com/scrapy/sphinx-scrapy.git@0.8.11

docs/requirements.txt

@@ -156,7 +156,7 @@ sphinx-rtd-theme==3.1.0
     # via
     #   -r docs/requirements.in
     #   sphinx-rtd-dark-mode
-sphinx-scrapy @ git+https://github.com/scrapy/sphinx-scrapy.git@fe176adc1a8577601bc3fa39b590ebed71a7e9b8
+sphinx-scrapy @ git+https://github.com/scrapy/sphinx-scrapy.git@6f8e5e0bbd171a857da480f7188f2a205041cb60
     # via -r docs/requirements.in
 sphinx-sitemap==2.9.0
     # via sphinx-scrapy

scrapy/pipelines/files.py

@@ -27,9 +27,7 @@ from twisted.internet.defer import Deferred, maybeDeferred
 from scrapy.exceptions import IgnoreRequest, NotConfigured, ScrapyDeprecationWarning
 from scrapy.http import Request, Response
 from scrapy.http.request import NO_CALLBACK
-from scrapy.pipelines.media import (
-    FileException as FileException,  # noqa: PLC0414  # re-exported for backward compatibility
-)
+from scrapy.pipelines.media import FileException as _FileException
 from scrapy.pipelines.media import (
     FileInfo,
     FileInfoOrError,
@@ -652,7 +650,7 @@ class FilesPipeline(MediaPipeline):
             f"{request} referred in <{referer}>: {failure.value}",
             extra={"spider": info.spider},
         )
-        raise FileException
+        raise _FileException
 
     async def media_downloaded(
         self,
@@ -671,7 +669,7 @@ class FilesPipeline(MediaPipeline):
                 {"status": response.status, "request": request, "referer": referer},
                 extra={"spider": info.spider},
             )
-            raise FileException("download-error")
+            raise _FileException("download-error")
 
         if not response.body:
             logger.warning(
@@ -680,7 +678,7 @@ class FilesPipeline(MediaPipeline):
                 {"request": request, "referer": referer},
                 extra={"spider": info.spider},
             )
-            raise FileException("empty-content")
+            raise _FileException("empty-content")
 
         status = "cached" if "cached" in response.flags else "downloaded"
         logger.debug(
@@ -696,7 +694,7 @@ class FilesPipeline(MediaPipeline):
             checksum: str = await ensure_awaitable(
                 self.file_downloaded(response, request, info, item=item)
             )
-        except FileException as exc:
+        except _FileException as exc:
             logger.warning(
                 "File (error): Error processing file from %(request)s "
                 "referred in <%(referer)s>: %(errormsg)s",
@@ -713,7 +711,7 @@ class FilesPipeline(MediaPipeline):
                 exc_info=True,
                 extra={"spider": info.spider},
             )
-            raise FileException(str(exc)) from exc
+            raise _FileException(str(exc)) from exc
 
         return {
             "url": request.url,
@@ -796,3 +794,15 @@ class FilesPipeline(MediaPipeline):
             if media_type:
                 media_ext = cast("str", mimetypes.guess_extension(media_type))
         return f"full/{media_guid}{media_ext}"
+
+
+def __getattr__(name: str) -> Any:
+    if name == "FileException":
+        warnings.warn(
+            "scrapy.pipelines.files.FileException is deprecated, use "
+            "scrapy.pipelines.media.FileException instead.",
+            ScrapyDeprecationWarning,
+            stacklevel=2,
+        )
+        return _FileException
+    raise AttributeError(f"module {__name__!r} has no attribute {name!r}")

scrapy/pipelines/images.py

@@ -18,7 +18,8 @@ from itemadapter import ItemAdapter
 from scrapy.exceptions import NotConfigured, ScrapyDeprecationWarning
 from scrapy.http import Request, Response
 from scrapy.http.request import NO_CALLBACK
-from scrapy.pipelines.files import FileException, FilesPipeline, _md5sum
+from scrapy.pipelines.files import FilesPipeline, _md5sum
+from scrapy.pipelines.media import FileException
 from scrapy.utils.defer import ensure_awaitable
 from scrapy.utils.python import to_bytes
 

scrapy/utils/log.py

@@ -2,7 +2,9 @@ from __future__ import annotations
 
 import logging
 import pprint
+import re
 import sys
+import warnings
 from collections.abc import MutableMapping
 from logging.config import dictConfig
 from typing import TYPE_CHECKING, Any, cast
@@ -12,6 +14,7 @@ from twisted.python import log as twisted_log
 from twisted.python.failure import Failure
 
 import scrapy
+from scrapy.exceptions import ScrapyDeprecationWarning
 from scrapy.settings import Settings
 from scrapy.utils.versions import get_versions
 
@@ -242,6 +245,9 @@ class LogCounterHandler(logging.Handler):
         self.crawler.stats.inc_value(sname)
 
 
+_MSG_MAPPING_PLACEHOLDER = re.compile(r"%\(\w+\)")
+
+
 def logformatter_adapter(
     logkws: LogFormatterResult,
 ) -> tuple[Any, ...]:
@@ -257,6 +263,20 @@ def logformatter_adapter(
     # argument, so empty args are left out. Tuple args become one positional
     # argument each, while a dict is a single positional argument.
     if not args:
+        if _MSG_MAPPING_PLACEHOLDER.search(message):
+            # The log formatter method has already returned, so there is no
+            # frame of it left in the stack to point at. msg is part of the
+            # warning message instead, so that each offending method gets its
+            # own warning.
+            warnings.warn(
+                f"A log formatter method returned msg {message!r} with "
+                f"%(name)s placeholders and no args. Interpolating msg with "
+                f"the returned dict is deprecated, return those values under "
+                f"args instead.",
+                ScrapyDeprecationWarning,
+                stacklevel=1,
+            )
+            return (level, message, logkws)
         return (level, message)
     if isinstance(args, tuple):
         return (level, message, *args)

tests/test_pipeline_files.py

@@ -28,15 +28,15 @@ from scrapy.crawler import Crawler
 from scrapy.exceptions import IgnoreRequest, NotConfigured, ScrapyDeprecationWarning
 from scrapy.http import Request, Response
 from scrapy.item import Field, Item
+from scrapy.pipelines import files
 from scrapy.pipelines.files import (
-    FileException,
     FilesPipeline,
     FSFilesStore,
     FTPFilesStore,
     GCSFilesStore,
     S3FilesStore,
 )
-from scrapy.pipelines.media import _MediaRequestFiltered
+from scrapy.pipelines.media import FileException, _MediaRequestFiltered
 from scrapy.settings import Settings
 from scrapy.utils.asyncio import call_later
 from scrapy.utils.defer import maybe_deferred_to_future
@@ -1342,3 +1342,11 @@ def test_files_pipeline_raises_notconfigured_when_files_store_invalid(store):
 
     with pytest.raises(NotConfigured):
         build_from_crawler(FilesPipeline, crawler)
+
+
+def test_file_exception_deprecated_import():
+    with pytest.warns(ScrapyDeprecationWarning, match="FileException"):
+        assert files.FileException is FileException
+
+    with pytest.raises(AttributeError):
+        files.nonexistent

tests/test_pipeline_media.py

@@ -10,8 +10,8 @@ from twisted.python.failure import Failure
 from scrapy import signals
 from scrapy.exceptions import ScrapyDeprecationWarning
 from scrapy.http import Request, Response
-from scrapy.pipelines.files import FileException
 from scrapy.pipelines.media import (
+    FileException,
     FileInfo,
     FileInfoOrError,
     MediaPipeline,

tests/test_utils_log.py

@@ -4,12 +4,14 @@ import json
 import logging
 import re
 import sys
+import warnings
 from io import StringIO
 from typing import TYPE_CHECKING, Any, cast
 
 import pytest
 from twisted.python.failure import Failure
 
+from scrapy.exceptions import ScrapyDeprecationWarning
 from scrapy.utils.log import (
     LogCounterHandler,
     SpiderLoggerAdapter,
@@ -332,7 +334,9 @@ class TestLogformatterAdapter:
             "LogFormatterResult",
             {"level": logging.INFO, "msg": "90% done", "args": args},
         )
-        assert self._log(caplog, logkws) == "90% done"
+        with warnings.catch_warnings():
+            warnings.simplefilter("error", ScrapyDeprecationWarning)
+            assert self._log(caplog, logkws) == "90% done"
 
     @pytest.mark.parametrize(
         ("msg", "args"),
@@ -345,4 +349,16 @@ class TestLogformatterAdapter:
         args: dict[str, Any] | tuple[Any, ...],
     ) -> None:
         logkws: LogFormatterResult = {"level": logging.INFO, "msg": msg, "args": args}
-        assert self._log(caplog, logkws) == "90% done"
+        with warnings.catch_warnings():
+            warnings.simplefilter("error", ScrapyDeprecationWarning)
+            assert self._log(caplog, logkws) == "90% done"
+
+    def test_msg_mapping_placeholders_without_args(
+        self, caplog: pytest.LogCaptureFixture
+    ) -> None:
+        logkws = cast(
+            "LogFormatterResult",
+            {"level": logging.INFO, "msg": "%(pct)d%% done", "pct": 90},
+        )
+        with pytest.warns(ScrapyDeprecationWarning, match="no args"):
+            assert self._log(caplog, logkws) == "90% done"

tox.ini

@@ -5,7 +5,7 @@
 
 [tox]
 requires =
-    sphinx-scrapy[tox] @ git+https://github.com/scrapy/sphinx-scrapy.git@0.8.10
+    sphinx-scrapy[tox] @ git+https://github.com/scrapy/sphinx-scrapy.git@0.8.11
     tox-uv
 envlist =
     pre-commit