From 2021b2c12a3f4acf6fd8b20f9c3dc42837e806f3 Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Wed, 10 Jun 2026 21:12:27 +0200 Subject: [PATCH] Solve typing issues, improve coverage --- conftest.py | 2 ++ docs/topics/settings.rst | 4 +-- pyproject.toml | 2 ++ scrapy/core/downloader/handlers/s3.py | 4 ++- scrapy/pipelines/files.py | 6 ++-- tests/test_utils_secrets.py | 50 +++++++++++++++++++++++++-- tox.ini | 6 ++++ 7 files changed, 65 insertions(+), 9 deletions(-) diff --git a/conftest.py b/conftest.py index cf0568111..e65301e3a 100644 --- a/conftest.py +++ b/conftest.py @@ -149,6 +149,8 @@ def pytest_runtest_setup(item): "botocore", "boto3", "mitmproxy", + "keyring", + "dotenv", ] for module in optional_deps: diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index 1c141f489..b5acbe6ab 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -263,7 +263,7 @@ JSON object with an ``"env"`` key: If the environment variable is not set, ``getsecret()`` logs a warning and the default setting value is used instead. -To read the secret from the system keyring (requires the keyring_ package), +To read the secret from the system keyring (requires keyring_ >= 21.7.0), use a ``"keyring"`` key. The value can be a username string — in which case the service name defaults to ``"scrapy"`` — or an object for full control: @@ -813,7 +813,7 @@ each crawl (via :class:`~scrapy.crawler.Crawler`). Relative paths are resolved from the current working directory. If the file does not exist the setting is silently ignored. -Loading a :file:`.env` file requires the python-dotenv_ package:: +Loading a :file:`.env` file requires python-dotenv_ >= 0.10.2:: pip install python-dotenv diff --git a/pyproject.toml b/pyproject.toml index 2d857dc6d..a4072d523 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -269,6 +269,8 @@ markers = [ "requires_botocore: marks tests that need botocore (but not boto3)", "requires_boto3: marks tests that need botocore and boto3", "requires_mitmproxy: marks tests that need mitmproxy", + "requires_keyring: marks tests that need keyring", + "requires_dotenv: marks tests that need python-dotenv", "requires_internet: marks tests that need real Internet access", ] filterwarnings = [ diff --git a/scrapy/core/downloader/handlers/s3.py b/scrapy/core/downloader/handlers/s3.py index a1ede0cfd..927b6bf0b 100644 --- a/scrapy/core/downloader/handlers/s3.py +++ b/scrapy/core/downloader/handlers/s3.py @@ -37,7 +37,9 @@ class S3DownloadHandler(BaseDownloadHandler): # botocore.auth.BaseSigner doesn't have an __init__() with args, only subclasses do self._signer = SignerCls( # type: ignore[call-arg] botocore.credentials.Credentials( - aws_access_key_id, aws_secret_access_key, aws_session_token + aws_access_key_id, + aws_secret_access_key, + aws_session_token, # type: ignore[arg-type] ) ) diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py index f856c3555..800288242 100644 --- a/scrapy/pipelines/files.py +++ b/scrapy/pipelines/files.py @@ -153,9 +153,9 @@ class FSFilesStore: class S3FilesStore: - AWS_ACCESS_KEY_ID = None - AWS_SECRET_ACCESS_KEY = None - AWS_SESSION_TOKEN = None + AWS_ACCESS_KEY_ID: str | None = None + AWS_SECRET_ACCESS_KEY: str | None = None + AWS_SESSION_TOKEN: str | None = None AWS_ENDPOINT_URL = None AWS_REGION_NAME = None AWS_USE_SSL = None diff --git a/tests/test_utils_secrets.py b/tests/test_utils_secrets.py index ee5bdced8..639738f81 100644 --- a/tests/test_utils_secrets.py +++ b/tests/test_utils_secrets.py @@ -230,8 +230,8 @@ class TestLoadDotenv: _load_dotenv(path=absent) assert dict(__import__("os").environ) == before + @pytest.mark.requires_dotenv def test_loads_variables(self, tmp_path): - pytest.importorskip("dotenv") dotenv_file = tmp_path / ".env" dotenv_file.write_text("SCRAPY_TEST_SECRET_VAR=hello\n") with mock.patch.dict("os.environ", {}, clear=False): @@ -241,8 +241,8 @@ class TestLoadDotenv: assert __import__("os").environ.get("SCRAPY_TEST_SECRET_VAR") == "hello" __import__("os").environ.pop("SCRAPY_TEST_SECRET_VAR", None) + @pytest.mark.requires_dotenv def test_real_env_wins_over_dotenv(self, tmp_path): - pytest.importorskip("dotenv") dotenv_file = tmp_path / ".env" dotenv_file.write_text("SCRAPY_TEST_OVERRIDE_VAR=from-file\n") with mock.patch.dict( @@ -251,8 +251,8 @@ class TestLoadDotenv: _load_dotenv(path=str(dotenv_file), override=False) assert __import__("os").environ["SCRAPY_TEST_OVERRIDE_VAR"] == "from-env" + @pytest.mark.requires_dotenv def test_override_true_replaces_env(self, tmp_path): - pytest.importorskip("dotenv") dotenv_file = tmp_path / ".env" dotenv_file.write_text("SCRAPY_TEST_OVERRIDE_VAR=from-file\n") with mock.patch.dict( @@ -261,3 +261,47 @@ class TestLoadDotenv: _load_dotenv(path=str(dotenv_file), override=True) assert __import__("os").environ["SCRAPY_TEST_OVERRIDE_VAR"] == "from-file" __import__("os").environ.pop("SCRAPY_TEST_OVERRIDE_VAR", None) + + +@pytest.mark.requires_keyring +class TestResolveSecretKeyringIntegration: + """Integration tests that exercise the real keyring library.""" + + def test_get_password_returns_value(self): + import keyring # noqa: PLC0415 + import keyring.backend # noqa: PLC0415 + import keyring.backends.null # noqa: PLC0415 + + class _InMemoryKeyring(keyring.backend.KeyringBackend): + priority = 1 + _store: dict[tuple[str, str], str] = {} + + def get_password(self, service, username): + return self._store.get((service, username)) + + def set_password(self, service, username, password): + self._store[(service, username)] = password + + def delete_password(self, service, username): + self._store.pop((service, username), None) + + backend = _InMemoryKeyring() + backend.set_password("scrapy", "my-account", "real-secret") + original = keyring.get_keyring() + try: + keyring.set_keyring(backend) + assert resolve_secret({"keyring": "my-account"}) == "real-secret" + finally: + keyring.set_keyring(original) + + def test_missing_entry_raises_key_error(self): + import keyring # noqa: PLC0415 + import keyring.backends.null # noqa: PLC0415 + + original = keyring.get_keyring() + try: + keyring.set_keyring(keyring.backends.null.Keyring()) + with pytest.raises(KeyError): + resolve_secret({"keyring": "nonexistent"}) + finally: + keyring.set_keyring(original) diff --git a/tox.ini b/tox.ini index e21e1f6cf..cc1b9dc95 100644 --- a/tox.ini +++ b/tox.ini @@ -47,6 +47,8 @@ deps = mypy==2.1.0 typing-extensions==4.15.0 Pillow==12.2.0 + keyring==22.3.0 + python-dotenv==0.10.2 Protego==0.6.0 Twisted==26.4.0 attrs==26.1.0 @@ -143,6 +145,8 @@ basepython = python3 deps = {[testenv]deps} Pillow + keyring + python-dotenv Twisted[http2] boto3 bpython # optional for shell wrapper tests @@ -160,6 +164,8 @@ basepython = {[pinned]basepython} deps = {[pinned]deps} Pillow==8.3.2 + keyring==21.7.0 + python-dotenv==0.10.2 Twisted[http2]==21.7.0 boto3==1.20.0 bpython==0.7.1