🐛Fixes TypeError: issubclass() arg 1 must be a class while building…

… BaseCustomSettings classes (#5253)

packages/pytest-simcore/src/pytest_simcore/environment_configs.py

@@ -8,7 +8,7 @@
 import pytest
 
 from .helpers.typing_env import EnvVarsDict
-from .helpers.utils_envs import load_dotenv, setenvs_from_dict
+from .helpers.utils_envs import delenvs_from_dict, load_dotenv, setenvs_from_dict
 
 
 @pytest.fixture(scope="session")
@@ -23,3 +23,16 @@ def mock_env_devel_environment(
     env_devel_dict: EnvVarsDict, monkeypatch: pytest.MonkeyPatch
 ) -> EnvVarsDict:
     return setenvs_from_dict(monkeypatch, env_devel_dict)
+
+
+@pytest.fixture
+def all_env_devel_undefined(
+    monkeypatch: pytest.MonkeyPatch, env_devel_dict: EnvVarsDict
+):
+    """Ensures that all env vars in .env-devel are undefined in the test environment
+
+    NOTE: this is useful to have a clean starting point and avoid
+    the environment to influence your test. I found this situation
+    when some script was accidentaly injecting the entire .env-devel in the environment
+    """
+    delenvs_from_dict(monkeypatch, env_devel_dict, raising=False)

packages/pytest-simcore/src/pytest_simcore/helpers/typing_env.py

@@ -1,7 +1,8 @@
+from collections.abc import Iterable
 from typing import TypeAlias
 
 EnvVarsDict: TypeAlias = dict[str, str]
-EnvVarsList: TypeAlias = list[str]
+EnvVarsIterable: TypeAlias = Iterable[str]
 
 
 # SEE packages/pytest-simcore/tests/test_helpers_utils_envs.py

packages/pytest-simcore/src/pytest_simcore/helpers/utils_envs.py

@@ -3,44 +3,42 @@
 """
 
 import os
-from copy import deepcopy
 from io import StringIO
 from pathlib import Path
 
 import dotenv
 import pytest
 
-from .typing_env import EnvVarsDict, EnvVarsList
+from .typing_env import EnvVarsDict, EnvVarsIterable
 
 #
 # monkeypatch using dict
 #
 
 
-def delenvs_from_dict(monkeypatch: pytest.MonkeyPatch, envs: EnvVarsList):
-    for var in envs:
-        assert isinstance(var, str)
-        assert var is not None  # None keys cannot be is defined w/o value
-        monkeypatch.delenv(var)
-
-
 def setenvs_from_dict(
-    monkeypatch: pytest.MonkeyPatch, envs: EnvVarsDict
+    monkeypatch: pytest.MonkeyPatch, envs: dict[str, str | bool]
 ) -> EnvVarsDict:
+    env_vars = {}
+
     for key, value in envs.items():
         assert isinstance(key, str)
-        assert (
-            value is not None
-        ), f"{key=},{value=}"  # None keys cannot be is defined w/o value
-        converted_value = value
+        assert value is not None, f"{key=},{value=}"
+
+        v = value
+
         if isinstance(value, bool):
-            converted_value = f"{'true' if value else 'false'}"
-        assert isinstance(
-            converted_value, str
-        ), f"client MUST explicitly stringify values since some cannot be done automatically e.g. json-like values. problematic {key=},{value=}"
+            v = "true" if value else "false"
+
+        assert isinstance(v, str), (
+            "caller MUST explicitly stringify values since some cannot be done automatically"
+            f"e.g. json-like values. Check {key=},{value=}"
+        )
 
-        monkeypatch.setenv(key, converted_value)
-    return deepcopy(envs)
+        monkeypatch.setenv(key, v)
+        env_vars[key] = v
+
+    return env_vars
 
 
 def load_dotenv(envfile_content_or_path: Path | str, **options) -> EnvVarsDict:
@@ -57,6 +55,17 @@ def load_dotenv(envfile_content_or_path: Path | str, **options) -> EnvVarsDict:
     return {k: v or "" for k, v in dotenv.dotenv_values(**kwargs).items()}
 
 
+def delenvs_from_dict(
+    monkeypatch: pytest.MonkeyPatch,
+    envs: EnvVarsIterable,
+    *,
+    raising: bool = True,
+) -> None:
+    for key in envs:
+        assert isinstance(key, str)
+        monkeypatch.delenv(key, raising)
+
+
 #
 # monkeypath using envfiles ('.env' and also denoted as dotfiles)
 #
@@ -76,7 +85,8 @@ def setenvs_from_envfile(
 def delenvs_from_envfile(
     monkeypatch: pytest.MonkeyPatch,
     content_or_path: str | Path,
-    raising: bool,
+    *,
+    raising: bool = True,
     **dotenv_kwags,
 ) -> EnvVarsDict:
     """Batch monkeypatch.delenv(...) on all env vars in an envfile"""

packages/settings-library/src/settings_library/base.py

@@ -1,9 +1,16 @@
 import logging
 from collections.abc import Sequence
 from functools import cached_property
-from typing import Final, get_args
-
-from pydantic import BaseConfig, BaseSettings, Extra, ValidationError, validator
+from typing import Final, get_args, get_origin
+
+from pydantic import (
+    BaseConfig,
+    BaseSettings,
+    ConfigError,
+    Extra,
+    ValidationError,
+    validator,
+)
 from pydantic.error_wrappers import ErrorList, ErrorWrapper
 from pydantic.fields import ModelField, Undefined
 
@@ -90,7 +97,12 @@ def prepare_field(cls, field: ModelField) -> None:
             if args := get_args(field_type):
                 field_type = next(a for a in args if a != type(None))
 
-            if issubclass(field_type, BaseCustomSettings):
+            # Avoids issubclass raising TypeError. SEE test_issubclass_type_error_with_pydantic_models
+            is_not_composed = (
+                get_origin(field_type) is None
+            )  # is not composed as dict[str, Any] or Generic[Base]
+
+            if is_not_composed and issubclass(field_type, BaseCustomSettings):
                 if auto_default_from_env:
                     assert field.field_info.default is Undefined
                     assert field.field_info.default_factory is None
@@ -100,13 +112,13 @@ def prepare_field(cls, field: ModelField) -> None:
                     field.default = None
                     field.required = False  # has a default now
 
-            elif issubclass(field_type, BaseSettings):
+            elif is_not_composed and issubclass(field_type, BaseSettings):
                 msg = f"{cls}.{field.name} of type {field_type} must inherit from BaseCustomSettings"
-                raise ValueError(msg)
+                raise ConfigError(msg)
 
             elif auto_default_from_env:
                 msg = f"auto_default_from_env=True can only be used in BaseCustomSettings subclassesbut field {cls}.{field.name} is {field_type} "
-                raise ValueError(msg)
+                raise ConfigError(msg)
 
     @classmethod
     def create_from_envs(cls, **overrides):

packages/settings-library/tests/conftest.py

@@ -4,7 +4,6 @@
 
 import sys
 from pathlib import Path
-from typing import Optional
 
 import pytest
 import settings_library
@@ -97,13 +96,9 @@ class _ApplicationSettings(BaseCustomSettings):
 
         # NOTE: by convention, an addon is disabled when APP_ADDON=None, so we make this
         # entry nullable as well
-        APP_OPTIONAL_ADDON: Optional[_ModuleSettings] = Field(
-            auto_default_from_env=True
-        )
+        APP_OPTIONAL_ADDON: _ModuleSettings | None = Field(auto_default_from_env=True)
 
         # NOTE: example of a group that cannot be disabled (not nullable)
-        APP_REQUIRED_PLUGIN: Optional[PostgresSettings] = Field(
-            auto_default_from_env=True
-        )
+        APP_REQUIRED_PLUGIN: PostgresSettings | None = Field(auto_default_from_env=True)
 
     return _ApplicationSettings

packages/settings-library/tests/test_base.py

@@ -3,12 +3,13 @@
 # pylint: disable=unused-variable
 # pylint: disable=too-many-arguments
 
+import inspect
 import json
-from typing import Any, Callable, Optional
+from typing import Any, Callable
 
 import pytest
 import settings_library.base
-from pydantic import BaseModel, ValidationError
+from pydantic import BaseModel, BaseSettings, ValidationError
 from pydantic.fields import Field
 from pytest import MonkeyPatch
 from pytest_mock import MockerFixture
@@ -62,13 +63,13 @@ class M1(BaseCustomSettings):
             VALUE_DEFAULT: S = S(S_VALUE=42)
             VALUE_CONFUSING: S = None  # type: ignore
 
-            VALUE_NULLABLE_REQUIRED: Optional[S] = ...  # type: ignore
-            VALUE_NULLABLE_OPTIONAL: Optional[S]
+            VALUE_NULLABLE_REQUIRED: S | None = ...  # type: ignore
+            VALUE_NULLABLE_OPTIONAL: S | None
 
-            VALUE_NULLABLE_DEFAULT_VALUE: Optional[S] = S(S_VALUE=42)
-            VALUE_NULLABLE_DEFAULT_NULL: Optional[S] = None
+            VALUE_NULLABLE_DEFAULT_VALUE: S | None = S(S_VALUE=42)
+            VALUE_NULLABLE_DEFAULT_NULL: S | None = None
 
-            VALUE_NULLABLE_DEFAULT_ENV: Optional[S] = Field(auto_default_from_env=True)
+            VALUE_NULLABLE_DEFAULT_ENV: S | None = Field(auto_default_from_env=True)
             VALUE_DEFAULT_ENV: S = Field(auto_default_from_env=True)
 
         class M2(BaseCustomSettings):
@@ -78,10 +79,10 @@ class M2(BaseCustomSettings):
             #
 
             # defaults disabled but only explicit enabled
-            VALUE_NULLABLE_DEFAULT_NULL: Optional[S] = None
+            VALUE_NULLABLE_DEFAULT_NULL: S | None = None
 
             # defaults enabled but if not exists, it disables
-            VALUE_NULLABLE_DEFAULT_ENV: Optional[S] = Field(auto_default_from_env=True)
+            VALUE_NULLABLE_DEFAULT_ENV: S | None = Field(auto_default_from_env=True)
 
             # cannot be disabled
             VALUE_DEFAULT_ENV: S = Field(auto_default_from_env=True)
@@ -200,8 +201,8 @@ def test_auto_default_to_none_logs_a_warning(
     S = create_settings_class("S")
 
     class SettingsClass(BaseCustomSettings):
-        VALUE_NULLABLE_DEFAULT_NULL: Optional[S] = None
-        VALUE_NULLABLE_DEFAULT_ENV: Optional[S] = Field(auto_default_from_env=True)
+        VALUE_NULLABLE_DEFAULT_NULL: S | None = None
+        VALUE_NULLABLE_DEFAULT_ENV: S | None = Field(auto_default_from_env=True)
 
     instance = SettingsClass.create_from_envs()
     assert instance.VALUE_NULLABLE_DEFAULT_NULL == None
@@ -222,8 +223,8 @@ def test_auto_default_to_not_none(
         S = create_settings_class("S")
 
         class SettingsClass(BaseCustomSettings):
-            VALUE_NULLABLE_DEFAULT_NULL: Optional[S] = None
-            VALUE_NULLABLE_DEFAULT_ENV: Optional[S] = Field(auto_default_from_env=True)
+            VALUE_NULLABLE_DEFAULT_NULL: S | None = None
+            VALUE_NULLABLE_DEFAULT_ENV: S | None = Field(auto_default_from_env=True)
 
         instance = SettingsClass.create_from_envs()
         assert instance.VALUE_NULLABLE_DEFAULT_NULL == None
@@ -260,12 +261,12 @@ def test_how_settings_parse_null_environs(monkeypatch: MonkeyPatch):
     }
 
     class SettingsClass(BaseCustomSettings):
-        VALUE_TO_NOTHING: Optional[str]
-        VALUE_TO_WORD_NULL: Optional[str]
-        VALUE_TO_WORD_NONE: Optional[str]
-        VALUE_TO_ZERO: Optional[str]
+        VALUE_TO_NOTHING: str | None
+        VALUE_TO_WORD_NULL: str | None
+        VALUE_TO_WORD_NONE: str | None
+        VALUE_TO_ZERO: str | None
 
-        INT_VALUE_TO_ZERO: Optional[int]
+        INT_VALUE_TO_ZERO: int | None
 
     instance = SettingsClass.create_from_envs()
 
@@ -278,7 +279,7 @@ class SettingsClass(BaseCustomSettings):
     )
 
     class SettingsClassExt(SettingsClass):
-        INT_VALUE_TO_NOTHING: Optional[int]
+        INT_VALUE_TO_NOTHING: int | None
 
     with pytest.raises(ValidationError) as err_info:
         SettingsClassExt.create_from_envs()
@@ -289,3 +290,35 @@ class SettingsClassExt(SettingsClass):
         "msg": "value is not a valid integer",
         "type": "type_error.integer",
     }
+
+
+def test_issubclass_type_error_with_pydantic_models():
+    # There is a problem
+    #
+    # TypeError: issubclass() arg 1 must be a class
+    #
+    # SEE https://github.com/pydantic/pydantic/issues/545
+    #
+    # >> issubclass(dict, BaseSettings)
+    # False
+    # >> issubclass(dict[str, str], BaseSettings)
+    # Traceback (most recent call last):
+    # File "<string>", line 1, in <module>
+    # File "/home/crespo/.pyenv/versions/3.10.13/lib/python3.10/abc.py", line 123, in __subclasscheck__
+    #     return _abc_subclasscheck(cls, subclass)
+    # TypeError: issubclass() arg 1 must be a class
+    #
+
+    assert inspect.isclass(dict[str, str])
+    assert not issubclass(dict, BaseSettings)
+
+    # NOTE: this should be fixed by pydantic at some point. When this happens, this test will fail
+    with pytest.raises(TypeError):
+        issubclass(dict[str, str], BaseSettings)
+
+    # here reproduces the problem with our settings that ANE and PC had
+    class SettingsClassThatFailed(BaseCustomSettings):
+        FOO: dict[str, str] | None = Field(default=None)
+
+    SettingsClassThatFailed(FOO={})
+    assert SettingsClassThatFailed(FOO=None) == SettingsClassThatFailed()