Make MinIO bucket creation opt-in with production-safe defaults.
PR Title Check / check-title (pull_request) Successful in 6s
Test Python Package / unit-tests (pull_request) Successful in 18s
Code Quality Pipeline / code-quality (pull_request) Successful in 1m28s
Test Python Package / integration-tests (pull_request) Failing after 1m54s
Test Python Package / coverage-report (pull_request) Has been skipped
PR Title Check / check-title (pull_request) Successful in 6s
Test Python Package / unit-tests (pull_request) Successful in 18s
Code Quality Pipeline / code-quality (pull_request) Successful in 1m28s
Test Python Package / integration-tests (pull_request) Failing after 1m54s
Test Python Package / coverage-report (pull_request) Has been skipped
Require buckets to exist by default on connect, extract env_bool parsing, and document local dev overrides for secure and auto-creation settings. Co-authored-by: Cursor <[email protected]>
This commit is contained in:
co-authored by
Cursor
parent
1431f455c1
commit
4a5b3b631f
@@ -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
|
||||
|
||||
@@ -49,15 +49,18 @@ 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_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
|
||||
)
|
||||
)
|
||||
```
|
||||
|
||||
@@ -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 config is None:
|
||||
if client is not None:
|
||||
if client is not None and config is None:
|
||||
raise ValueError("config is required when client is provided")
|
||||
if config is None:
|
||||
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
|
||||
|
||||
@@ -35,9 +35,9 @@ class RedisAdapter(JsonRepositoryInterface, ConnectionAwareAdapter):
|
||||
client: redis.Redis | None = None,
|
||||
) -> None:
|
||||
super().__init__()
|
||||
if config is None:
|
||||
if client is not None:
|
||||
if client is not None and config is None:
|
||||
raise ValueError("config is required when client is provided")
|
||||
if config is None:
|
||||
config = RedisConfig.from_env(self.uri_env_var_name)
|
||||
self._config = config
|
||||
self._client_injected = client is not None
|
||||
|
||||
@@ -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}")
|
||||
@@ -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,
|
||||
),
|
||||
)
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
|
||||
@@ -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)
|
||||
@@ -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)
|
||||
|
||||
@@ -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(
|
||||
|
||||
Reference in New Issue
Block a user