🎨Storage: disallow optional S3/postgres settings (#5701)

Author: sanderegg

services/clusters-keeper/tests/unit/conftest.py

@@ -255,8 +255,10 @@ def clusters_keeper_docker_compose_file(installed_package_dir: Path) -> Path:
 
 @pytest.fixture
 def clusters_keeper_docker_compose() -> dict[str, Any]:
-    data = importlib.resources.read_text(
-        simcore_service_clusters_keeper.data, "docker-compose.yml"
+    data = (
+        importlib.resources.files(simcore_service_clusters_keeper.data)
+        .joinpath("docker-compose.yml")
+        .read_text()
     )
     assert data
     return yaml.safe_load(data)

services/storage/.env-devel

@@ -23,6 +23,7 @@ S3_BUCKET_NAME=simcore
 # 172.17.0.1 is the docker0 interface, which redirect from inside a container onto the host network interface.
 S3_ENDPOINT=http://172.17.0.1:9001
 S3_SECRET_KEY=12345678
+S3_REGION=us-east-1
 
 BF_API_SECRET=none
 BF_API_KEY=none

services/storage/src/simcore_service_storage/application.py

@@ -57,21 +57,20 @@ def create(settings: Settings) -> web.Application:
             skip_routes=None,
         )
 
-    if settings.STORAGE_POSTGRES:
-        setup_db(app)  # -> postgres service
-    if settings.STORAGE_S3:
-        setup_s3(app)  # -> minio service
+    setup_db(app)
+    setup_s3(app)
 
     setup_long_running_tasks(app)
     setup_rest(app)
-    setup_redis(app)
 
-    if settings.STORAGE_POSTGRES and settings.STORAGE_S3:
-        setup_dsm(app)  # core subsystem. Needs s3 and db setups done
-        if settings.STORAGE_CLEANER_INTERVAL_S:
-            setup_dsm_cleaner(app)
+    if settings.STORAGE_REDIS:
+        setup_redis(app)
 
-        app.middlewares.append(dsm_exception_handler)
+    setup_dsm(app)
+    if settings.STORAGE_CLEANER_INTERVAL_S:
+        setup_dsm_cleaner(app)
+
+    app.middlewares.append(dsm_exception_handler)
 
     if settings.LOG_LEVEL == "DEBUG":
         setup_dev_error_logger(app)

services/storage/src/simcore_service_storage/settings.py

@@ -1,4 +1,6 @@
-from pydantic import Field, PositiveInt, validator
+from typing import Any
+
+from pydantic import Field, PositiveInt, root_validator, validator
 from settings_library.base import BaseCustomSettings
 from settings_library.basic_types import LogLevel, PortInt
 from settings_library.postgres import PostgresSettings
@@ -32,11 +34,11 @@ class Settings(BaseCustomSettings, MixinLoggingSettings):
         None, description="Pennsieve API secret ONLY for testing purposes"
     )
 
-    STORAGE_POSTGRES: PostgresSettings | None = Field(auto_default_from_env=True)
+    STORAGE_POSTGRES: PostgresSettings = Field(auto_default_from_env=True)
 
     STORAGE_REDIS: RedisSettings | None = Field(auto_default_from_env=True)
 
-    STORAGE_S3: S3Settings | None = Field(auto_default_from_env=True)
+    STORAGE_S3: S3Settings = Field(auto_default_from_env=True)
 
     STORAGE_TRACING: TracingSettings | None = Field(auto_default_from_env=True)
 
@@ -71,3 +73,12 @@ class Settings(BaseCustomSettings, MixinLoggingSettings):
     def _validate_loglevel(cls, value) -> str:
         log_level: str = cls.validate_log_level(value)
         return log_level
+
+    @root_validator()
+    @classmethod
+    def ensure_settings_consistency(cls, values: dict[str, Any]):
+        if values.get("STORAGE_CLEANER_INTERVAL_S") and not values.get("STORAGE_REDIS"):
+            raise ValueError(
+                "STORAGE_CLEANER_INTERVAL_S cleaner cannot be set without STORAGE_REDIS! Please correct settings."
+            )
+        return values

services/storage/tests/conftest.py

@@ -118,15 +118,17 @@ def project_slug_dir(osparc_simcore_root_dir) -> Path:
 
 
 @pytest.fixture(scope="session")
-def project_env_devel_dict(project_slug_dir: Path) -> dict:
+def project_env_devel_dict(project_slug_dir: Path) -> dict[str, str]:
     env_devel_file = project_slug_dir / ".env-devel"
     assert env_devel_file.exists()
     environ = dotenv.dotenv_values(env_devel_file, verbose=True, interpolate=True)
     return environ
 
 
 @pytest.fixture
-def project_env_devel_environment(project_env_devel_dict, monkeypatch) -> None:
+def project_env_devel_environment(
+    project_env_devel_dict: dict[str, str], monkeypatch: pytest.MonkeyPatch
+) -> None:
     for key, value in project_env_devel_dict.items():
         monkeypatch.setenv(key, value)
 
@@ -186,13 +188,13 @@ def mock_config(
     postgres_host_config: dict[str, str],
     mocked_s3_server_envs,
     datcore_adapter_service_mock: aioresponses.aioresponses,
-):
+) -> None:
     # NOTE: this can be overriden in tests that do not need all dependencies up
     ...
 
 
 @pytest.fixture
-def app_settings(mock_config) -> Settings:
+def app_settings(mock_config: None) -> Settings:
     test_app_settings = Settings.create_from_envs()
     print(f"{test_app_settings.json(indent=2)=}")
     return test_app_settings

services/storage/tests/unit/test_s3.py

@@ -3,42 +3,22 @@
 # pylint: disable=unused-variable
 
 
-import pytest
 from aiohttp.test_utils import TestClient
 from simcore_service_storage.s3 import get_s3_client
 from simcore_service_storage.settings import Settings
 
+pytest_simcore_core_services_selection = ["postgres"]
 
-@pytest.fixture(params=[True, False])
-def enable_s3(request: pytest.FixtureRequest) -> bool:
-    return request.param  # type: ignore
 
-
-@pytest.fixture
-def mock_config(
-    mocked_s3_server_envs, monkeypatch: pytest.MonkeyPatch, enable_s3: bool
-):
-    # NOTE: override services/storage/tests/conftest.py::mock_config
-    monkeypatch.setenv("STORAGE_POSTGRES", "null")
-    if not enable_s3:
-        # disable S3
-        monkeypatch.setenv("STORAGE_S3", "null")
-
-
-async def test_s3_client(enable_s3: bool, app_settings: Settings, client: TestClient):
+async def test_s3_client(app_settings: Settings, client: TestClient):
     assert client.app
-    if enable_s3:
-        assert app_settings.STORAGE_S3
-        s3_client = get_s3_client(client.app)
-        assert s3_client
-
-        response = await s3_client.client.list_buckets()
-        assert response
-        assert "Buckets" in response
-        assert len(response["Buckets"]) == 1
-        assert "Name" in response["Buckets"][0]
-        assert response["Buckets"][0]["Name"] == app_settings.STORAGE_S3.S3_BUCKET_NAME
-    else:
-        assert not app_settings.STORAGE_S3
-        with pytest.raises(KeyError):
-            get_s3_client(client.app)
+    assert app_settings.STORAGE_S3
+    s3_client = get_s3_client(client.app)
+    assert s3_client
+
+    response = await s3_client.client.list_buckets()
+    assert response
+    assert "Buckets" in response
+    assert len(response["Buckets"]) == 1
+    assert "Name" in response["Buckets"][0]
+    assert response["Buckets"][0]["Name"] == app_settings.STORAGE_S3.S3_BUCKET_NAME

services/storage/tests/unit/test_s3_client.py

@@ -30,7 +30,6 @@
 from models_library.projects_nodes_io import SimcoreS3FileID
 from pydantic import ByteSize, parse_obj_as
 from pytest_mock import MockFixture
-from pytest_simcore.helpers.utils_envs import EnvVarsDict
 from pytest_simcore.helpers.utils_parametrizations import byte_size_ids
 from simcore_service_storage.models import MultiPartUploadLinks, S3BucketName
 from simcore_service_storage.s3_client import (
@@ -46,13 +45,7 @@
 
 DEFAULT_EXPIRATION_SECS: Final[int] = 10
 
-
-@pytest.fixture
-def mock_config(
-    mocked_s3_server_envs: EnvVarsDict, monkeypatch: pytest.MonkeyPatch
-) -> None:
-    # NOTE: override services/storage/tests/conftest.py::mock_config
-    monkeypatch.setenv("STORAGE_POSTGRES", "null")
+pytest_simcore_core_services_selection = ["postgres"]
 
 
 async def test_storage_storage_s3_client_creation(

tests/environment-setup/test_used_python.py

@@ -35,8 +35,7 @@ def make_versions_comparable(*versions) -> list[tuple[int]]:
     for v in versions:
         if isinstance(v, str):
             v = to_version(v)
-        if len(v) < n:
-            n = len(v)
+        n = min(n, len(v))
         vers.append(v)
     return [v[:n] for v in vers]