diff --git a/.env.example b/.env.example index f5427fc..15963df 100644 --- a/.env.example +++ b/.env.example @@ -7,3 +7,4 @@ MINIO_ACCESS_KEY=minioadmin MINIO_SECRET_KEY=minioadmin MINIO_BUCKET=my-bucket MINIO_SECURE=false +MINIO_CREATE_BUCKET_IF_MISSING=true diff --git a/README.md b/README.md index 6149a2c..9ef4c3b 100644 --- a/README.md +++ b/README.md @@ -48,16 +48,19 @@ for key in repo.scan_keys("user:*"): ### MinIO (`ObjectRepositoryInterface`) -| Environment variable | Description | -| -------------------- | ------------------------------------------- | -| `MINIO_ENDPOINT` | MinIO server endpoint | -| `MINIO_ACCESS_KEY` | Access key | -| `MINIO_SECRET_KEY` | Secret key | -| `MINIO_BUCKET` | Bucket name (created on connect if missing) | -| `MINIO_SECURE` | Use HTTPS (`true`/`false`; default: `true`) | +| Environment variable | Description | +| -------------------------------- | ------------------------------------------------------------------------ | +| `MINIO_ENDPOINT` | MinIO server endpoint | +| `MINIO_ACCESS_KEY` | Access key | +| `MINIO_SECRET_KEY` | Secret key | +| `MINIO_BUCKET` | Bucket name | +| `MINIO_SECURE` | Use HTTPS (`true`/`false`; default: `true`) | +| `MINIO_CREATE_BUCKET_IF_MISSING` | Auto-create `MINIO_BUCKET` on connect (`true`/`false`; default: `false`) | Copy [`.env.example`](.env.example) to `.env` for local development. `RedisConfig.from_env()` and `MinioConfig.from_env()` load `.env` automatically when resolving configuration from the environment. +For production, it is recommended to leave `MINIO_CREATE_BUCKET_IF_MISSING` unset so that `connect()` fails fast if the expected bucket is missing. For local development, you will often want `MINIO_SECURE=false` and `MINIO_CREATE_BUCKET_IF_MISSING=true`. + ## Configuration injection Adapters accept optional `config` and `client` keyword arguments for explicit setup and testing: @@ -73,6 +76,7 @@ minio = MinioAdapter( secret_key="minioadmin", bucket="my-bucket", secure=False, + # create_bucket_if_missing=True, # convenient for local dev ) ) ``` diff --git a/python_repositories/adapters/minio_adapter.py b/python_repositories/adapters/minio_adapter.py index 64c8b2e..64aa55a 100644 --- a/python_repositories/adapters/minio_adapter.py +++ b/python_repositories/adapters/minio_adapter.py @@ -26,6 +26,7 @@ class MinioAdapter(ObjectRepositoryInterface, ConnectionAwareAdapter): secret_key_env_var_name: str = "MINIO_SECRET_KEY" bucket_env_var_name: str = "MINIO_BUCKET" secure_env_var_name: str = "MINIO_SECURE" + create_bucket_if_missing_env_var_name: str = "MINIO_CREATE_BUCKET_IF_MISSING" chunk_size: int = 5 * 2**20 # 5 MiB connection_name: str = "Minio" @@ -36,15 +37,16 @@ class MinioAdapter(ObjectRepositoryInterface, ConnectionAwareAdapter): client: minio.Minio | None = None, ) -> None: super().__init__() + if client is not None and config is None: + raise ValueError("config is required when client is provided") if config is None: - if client is not None: - raise ValueError("config is required when client is provided") config = MinioConfig.from_env( self.endpoint_env_var_name, self.access_key_env_var_name, self.secret_key_env_var_name, self.bucket_env_var_name, self.secure_env_var_name, + self.create_bucket_if_missing_env_var_name, ) self._config = config self._client_injected = client is not None @@ -88,6 +90,10 @@ class MinioAdapter(ObjectRepositoryInterface, ConnectionAwareAdapter): except Exception as exc: # pylint: disable=broad-except raise ConnectionError(f"Could not connect to Minio at {endpoint}") from exc if not client.bucket_exists(bucket): + if not self._config.create_bucket_if_missing: + raise ConnectionError( + f"Bucket '{bucket}' does not exist on Minio at {endpoint}" + ) self.logger.info(f"Creating bucket '{bucket}'") client.make_bucket(bucket) self._client = client diff --git a/python_repositories/adapters/redis_adapter.py b/python_repositories/adapters/redis_adapter.py index 4091c98..aee435f 100644 --- a/python_repositories/adapters/redis_adapter.py +++ b/python_repositories/adapters/redis_adapter.py @@ -35,9 +35,9 @@ class RedisAdapter(JsonRepositoryInterface, ConnectionAwareAdapter): client: redis.Redis | None = None, ) -> None: super().__init__() + if client is not None and config is None: + raise ValueError("config is required when client is provided") if config is None: - if client is not None: - raise ValueError("config is required when client is provided") config = RedisConfig.from_env(self.uri_env_var_name) self._config = config self._client_injected = client is not None diff --git a/python_repositories/config/env_bool.py b/python_repositories/config/env_bool.py new file mode 100644 index 0000000..db44400 --- /dev/null +++ b/python_repositories/config/env_bool.py @@ -0,0 +1,21 @@ +"""Parse boolean values from environment variables.""" + +from __future__ import annotations + +import os + +_TRUTHY = frozenset({"1", "true", "yes", "on"}) +_FALSY = frozenset({"0", "false", "no", "off"}) + + +def env_bool(name: str, default: bool) -> bool: + """Parse an environment variable as a boolean value.""" + raw = os.getenv(name) + if raw is None: + return default + normalized = raw.strip().lower() + if normalized in _TRUTHY: + return True + if normalized in _FALSY: + return False + raise ValueError(f"Invalid boolean value for {name}: {raw!r}") diff --git a/python_repositories/config/minio_config.py b/python_repositories/config/minio_config.py index 0405ddb..050d04d 100644 --- a/python_repositories/config/minio_config.py +++ b/python_repositories/config/minio_config.py @@ -8,21 +8,7 @@ from dataclasses import dataclass from python_utils import check_env from python_repositories.config.dotenv_loader import load_dotenv - -_TRUTHY = frozenset({"1", "true", "yes", "on"}) -_FALSY = frozenset({"0", "false", "no", "off"}) - - -def _env_bool(name: str, default: bool) -> bool: - raw = os.getenv(name) - if raw is None: - return default - normalized = raw.strip().lower() - if normalized in _TRUTHY: - return True - if normalized in _FALSY: - return False - raise ValueError(f"Invalid boolean value for {name}: {raw!r}") +from python_repositories.config.env_bool import env_bool @dataclass(frozen=True) @@ -34,6 +20,7 @@ class MinioConfig: secret_key: str bucket: str secure: bool = True + create_bucket_if_missing: bool = False @classmethod def from_env( @@ -43,6 +30,7 @@ class MinioConfig: secret_key_env_var_name: str = "MINIO_SECRET_KEY", bucket_env_var_name: str = "MINIO_BUCKET", secure_env_var_name: str = "MINIO_SECURE", + create_bucket_if_missing_env_var_name: str = ("MINIO_CREATE_BUCKET_IF_MISSING"), *, use_dotenv: bool = True, ) -> MinioConfig: @@ -61,5 +49,9 @@ class MinioConfig: access_key=str(os.getenv(access_key_env_var_name)), secret_key=str(os.getenv(secret_key_env_var_name)), bucket=str(os.getenv(bucket_env_var_name)), - secure=_env_bool(secure_env_var_name, default=True), + secure=env_bool(secure_env_var_name, default=True), + create_bucket_if_missing=env_bool( + create_bucket_if_missing_env_var_name, + default=False, + ), ) diff --git a/tests/integration/minio_adapter_test.py b/tests/integration/minio_adapter_test.py index 5cb56ca..770ac36 100644 --- a/tests/integration/minio_adapter_test.py +++ b/tests/integration/minio_adapter_test.py @@ -1,6 +1,7 @@ """Integration tests for the MinioAdapter.""" from collections.abc import Generator +from dataclasses import replace import logging import random from io import BytesIO @@ -118,17 +119,37 @@ def test_should_raise_connection_error_when_unable_to_connect() -> None: assert not adapter.is_connected() -def test_should_log_info_when_creating_expected_bucket( +def test_connect_raises_when_bucket_missing( + raw_minio_client: Minio, + minio_config: MinioConfig, +) -> None: + """Test that connect fails when the configured bucket is missing.""" + bucket_name = minio_config.bucket + raw_minio_client.remove_bucket(bucket_name) + adapter = MinioAdapter(config=minio_config) + + try: + with pytest.raises(ConnectionError, match="does not exist"): + adapter.connect() + assert not adapter.is_connected() + finally: + raw_minio_client.make_bucket(bucket_name) + + +def test_connect_creates_bucket_when_create_bucket_if_missing_enabled( raw_minio_client: Minio, minio_config: MinioConfig, caplog: pytest.LogCaptureFixture, ) -> None: - """Test that the MinioAdapter logs info when creating the expected bucket.""" + """Test that connect can create the configured bucket when enabled.""" bucket_name = minio_config.bucket raw_minio_client.remove_bucket(bucket_name) - adapter = MinioAdapter(config=minio_config) + config = replace(minio_config, create_bucket_if_missing=True) + adapter = MinioAdapter(config=config) + with caplog.at_level(logging.INFO): adapter.connect() + assert f"Creating bucket '{bucket_name}'" in caplog.text diff --git a/tests/unit/env_bool_test.py b/tests/unit/env_bool_test.py new file mode 100644 index 0000000..6d1870f --- /dev/null +++ b/tests/unit/env_bool_test.py @@ -0,0 +1,43 @@ +"""Unit tests for env_bool.""" + +from __future__ import annotations + +import pytest + +from python_repositories.config.env_bool import env_bool + + +@pytest.mark.parametrize( + ("value", "expected"), + [ + ("true", True), + ("1", True), + ("yes", True), + ("on", True), + ("false", False), + ("0", False), + ("no", False), + ("off", False), + ], +) +def test_env_bool_parses_truthy_and_falsy( + monkeypatch: pytest.MonkeyPatch, + value: str, + expected: bool, +) -> None: + monkeypatch.setenv("TEST_BOOL", value) + assert env_bool("TEST_BOOL", default=not expected) is expected + + +def test_env_bool_returns_default_when_unset(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv("TEST_BOOL", raising=False) + assert env_bool("TEST_BOOL", default=True) is True + assert env_bool("TEST_BOOL", default=False) is False + + +def test_env_bool_raises_for_invalid_value( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv("TEST_BOOL", "not-a-bool") + with pytest.raises(ValueError, match="TEST_BOOL"): + env_bool("TEST_BOOL", default=False) diff --git a/tests/unit/minio_adapter_test.py b/tests/unit/minio_adapter_test.py index 3de59b6..bbb6e70 100644 --- a/tests/unit/minio_adapter_test.py +++ b/tests/unit/minio_adapter_test.py @@ -2,6 +2,7 @@ from __future__ import annotations +from dataclasses import replace from unittest.mock import MagicMock import pytest @@ -94,6 +95,44 @@ def test_connect_disconnects_before_reconnect( new_client.list_buckets.assert_called_once() +def test_connect_raises_when_bucket_missing( + monkeypatch: pytest.MonkeyPatch, +) -> None: + mock_client = MagicMock(spec=Minio) + mock_client.bucket_exists.return_value = False + + monkeypatch.setattr( + "python_repositories.adapters.minio_adapter.minio.Minio", + lambda *args, **kwargs: mock_client, + ) + + adapter = MinioAdapter(config=TEST_MINIO_CONFIG) + + with pytest.raises(ConnectionError, match="does not exist"): + adapter.connect() + + mock_client.make_bucket.assert_not_called() + + +def test_connect_creates_bucket_when_create_bucket_if_missing_enabled( + monkeypatch: pytest.MonkeyPatch, +) -> None: + mock_client = MagicMock(spec=Minio) + mock_client.bucket_exists.return_value = False + config = replace(TEST_MINIO_CONFIG, create_bucket_if_missing=True) + + monkeypatch.setattr( + "python_repositories.adapters.minio_adapter.minio.Minio", + lambda *args, **kwargs: mock_client, + ) + + adapter = MinioAdapter(config=config) + adapter.connect() + + mock_client.bucket_exists.assert_called_once_with(config.bucket) + mock_client.make_bucket.assert_called_once_with(config.bucket) + + def test_get_closes_response_on_success() -> None: """get() must close and release the get_object HTTP response.""" mock_client = MagicMock(spec=Minio) diff --git a/tests/unit/minio_config_test.py b/tests/unit/minio_config_test.py index e530c6f..273bb67 100644 --- a/tests/unit/minio_config_test.py +++ b/tests/unit/minio_config_test.py @@ -22,6 +22,7 @@ def test_from_env_loads_all_fields(monkeypatch: pytest.MonkeyPatch) -> None: assert config.secret_key == "secret" assert config.bucket == "my-bucket" assert config.secure is True + assert config.create_bucket_if_missing is False def test_from_env_defaults_secure_to_true_when_unset( @@ -33,33 +34,27 @@ def test_from_env_defaults_secure_to_true_when_unset( assert config.secure is True -@pytest.mark.parametrize( - ("value", "expected"), - [ - ("true", True), - ("1", True), - ("yes", True), - ("false", False), - ("0", False), - ("no", False), - ], -) -def test_from_env_parses_secure( +def test_from_env_reads_secure_from_env(monkeypatch: pytest.MonkeyPatch) -> None: + _set_required_minio_env(monkeypatch) + monkeypatch.setenv("MINIO_SECURE", "false") + assert MinioConfig.from_env(use_dotenv=False).secure is False + + +def test_from_env_reads_create_bucket_if_missing_from_env( monkeypatch: pytest.MonkeyPatch, - value: str, - expected: bool, ) -> None: _set_required_minio_env(monkeypatch) - monkeypatch.setenv("MINIO_SECURE", value) - config = MinioConfig.from_env(use_dotenv=False) - assert config.secure is expected + monkeypatch.setenv("MINIO_CREATE_BUCKET_IF_MISSING", "true") + assert MinioConfig.from_env(use_dotenv=False).create_bucket_if_missing is True -def test_from_env_raises_for_invalid_secure(monkeypatch: pytest.MonkeyPatch) -> None: +def test_from_env_defaults_create_bucket_if_missing_to_false_when_unset( + monkeypatch: pytest.MonkeyPatch, +) -> None: _set_required_minio_env(monkeypatch) - monkeypatch.setenv("MINIO_SECURE", "not-a-bool") - with pytest.raises(ValueError, match="MINIO_SECURE"): - MinioConfig.from_env(use_dotenv=False) + monkeypatch.delenv("MINIO_CREATE_BUCKET_IF_MISSING", raising=False) + config = MinioConfig.from_env(use_dotenv=False) + assert config.create_bucket_if_missing is False def test_from_env_raises_when_endpoint_missing(