From e75711a65550198a9f9c8157a79e99f574790e30 Mon Sep 17 00:00:00 2001 From: Julian Vanden Broeck Date: Thu, 7 Nov 2024 16:34:08 +0100 Subject: [PATCH 1/3] Test raised ValueError on invalid settings file --- tests/test_source_json.py | 28 ++++++++++++++++++++++++++++ tests/test_source_toml.py | 24 ++++++++++++++++++++++++ tests/test_source_yaml.py | 24 ++++++++++++++++++++++++ 3 files changed, 76 insertions(+) diff --git a/tests/test_source_json.py b/tests/test_source_json.py index e348a6b..94d268f 100644 --- a/tests/test_source_json.py +++ b/tests/test_source_json.py @@ -5,6 +5,7 @@ import json from typing import Tuple, Type, Union +import pytest from pydantic import BaseModel from pydantic_settings import ( @@ -67,6 +68,33 @@ def settings_customise_sources( assert s.model_dump() == {} +def test_nondict_json_file(tmp_path): + p = tmp_path / '.env' + p.write_text( + """ + "noway" + """ + ) + + class Settings(BaseSettings): + foobar: str + model_config = SettingsConfigDict(json_file=p) + + @classmethod + def settings_customise_sources( + cls, + settings_cls: Type[BaseSettings], + init_settings: PydanticBaseSettingsSource, + env_settings: PydanticBaseSettingsSource, + dotenv_settings: PydanticBaseSettingsSource, + file_secret_settings: PydanticBaseSettingsSource, + ) -> Tuple[PydanticBaseSettingsSource, ...]: + return (JsonConfigSettingsSource(settings_cls),) + + with pytest.raises(ValueError): + Settings() + + def test_multiple_file_json(tmp_path): p5 = tmp_path / '.env.json5' p6 = tmp_path / '.env.json6' diff --git a/tests/test_source_toml.py b/tests/test_source_toml.py index 2918601..124520e 100644 --- a/tests/test_source_toml.py +++ b/tests/test_source_toml.py @@ -77,6 +77,30 @@ def settings_customise_sources( assert s.model_dump() == {} +@pytest.mark.skipif(sys.version_info <= (3, 11) and tomli is None, reason='tomli/tomllib is not installed') +def test_pyproject_nondict_toml(cd_tmp_path): + pyproject = cd_tmp_path / 'pyproject.toml' + pyproject.write_text( + """ + [tool.pydantic-settings] + foobar + """ + ) + + class Settings(BaseSettings): + foobar: str + model_config = SettingsConfigDict() + + @classmethod + def settings_customise_sources( + cls, settings_cls: Type[BaseSettings], **_kwargs: PydanticBaseSettingsSource + ) -> Tuple[PydanticBaseSettingsSource, ...]: + return (TomlConfigSettingsSource(settings_cls, pyproject),) + + with pytest.raises(ValueError): + Settings() + + @pytest.mark.skipif(sys.version_info <= (3, 11) and tomli is None, reason='tomli/tomllib is not installed') def test_multiple_file_toml(tmp_path): p1 = tmp_path / '.env.toml1' diff --git a/tests/test_source_yaml.py b/tests/test_source_yaml.py index fd25de6..d17f32c 100644 --- a/tests/test_source_yaml.py +++ b/tests/test_source_yaml.py @@ -85,6 +85,30 @@ def settings_customise_sources( assert s.nested.nested_field == 'world!' +@pytest.mark.skipif(yaml is None, reason='pyYaml is not installed') +def test_nondict_yaml_file(tmp_path): + p = tmp_path / '.env' + p.write_text('test invalid yaml') + + class Settings(BaseSettings): + foobar: str + model_config = SettingsConfigDict(yaml_file=p) + + @classmethod + def settings_customise_sources( + cls, + settings_cls: Type[BaseSettings], + init_settings: PydanticBaseSettingsSource, + env_settings: PydanticBaseSettingsSource, + dotenv_settings: PydanticBaseSettingsSource, + file_secret_settings: PydanticBaseSettingsSource, + ) -> Tuple[PydanticBaseSettingsSource, ...]: + return (YamlConfigSettingsSource(settings_cls),) + + with pytest.raises(ValueError): + Settings() + + @pytest.mark.skipif(yaml is None, reason='pyYaml is not installed') def test_yaml_no_file(): class Settings(BaseSettings): From 0225b19473f9ddd5b20806aaf5d3d8099d6c60f8 Mon Sep 17 00:00:00 2001 From: Julian Vanden Broeck Date: Tue, 1 Oct 2024 09:04:17 +0200 Subject: [PATCH 2/3] Use SettingsErrors for invalid file source Some file formats (at least yaml) allow to represent non dictionnary values. In such situation we can't add the values read from those files. Instead of raising a ValueError, we now raise a SettingsError and indicate we can't parse the related config file. --- pydantic_settings/sources.py | 6 +++++- tests/test_source_json.py | 3 ++- tests/test_source_toml.py | 3 ++- tests/test_source_yaml.py | 3 ++- 4 files changed, 11 insertions(+), 4 deletions(-) diff --git a/pydantic_settings/sources.py b/pydantic_settings/sources.py index 919f836..6c43994 100644 --- a/pydantic_settings/sources.py +++ b/pydantic_settings/sources.py @@ -1917,7 +1917,11 @@ def _read_files(self, files: PathType | None) -> dict[str, Any]: for file in files: file_path = Path(file).expanduser() if file_path.is_file(): - vars.update(self._read_file(file_path)) + try: + settings = self._read_file(file_path) + except ValueError as e: + raise SettingsError(f'Failed to parse settings from {file_path}, {e}') + vars.update(settings) return vars @abstractmethod diff --git a/tests/test_source_json.py b/tests/test_source_json.py index 94d268f..7a6ba5d 100644 --- a/tests/test_source_json.py +++ b/tests/test_source_json.py @@ -13,6 +13,7 @@ JsonConfigSettingsSource, PydanticBaseSettingsSource, SettingsConfigDict, + SettingsError, ) @@ -91,7 +92,7 @@ def settings_customise_sources( ) -> Tuple[PydanticBaseSettingsSource, ...]: return (JsonConfigSettingsSource(settings_cls),) - with pytest.raises(ValueError): + with pytest.raises(SettingsError, match='Failed to parse settings from .*, expecting an object'): Settings() diff --git a/tests/test_source_toml.py b/tests/test_source_toml.py index 124520e..0d2f70e 100644 --- a/tests/test_source_toml.py +++ b/tests/test_source_toml.py @@ -12,6 +12,7 @@ BaseSettings, PydanticBaseSettingsSource, SettingsConfigDict, + SettingsError, TomlConfigSettingsSource, ) @@ -97,7 +98,7 @@ def settings_customise_sources( ) -> Tuple[PydanticBaseSettingsSource, ...]: return (TomlConfigSettingsSource(settings_cls, pyproject),) - with pytest.raises(ValueError): + with pytest.raises(SettingsError, match='Failed to parse settings from'): Settings() diff --git a/tests/test_source_yaml.py b/tests/test_source_yaml.py index d17f32c..10f5fd1 100644 --- a/tests/test_source_yaml.py +++ b/tests/test_source_yaml.py @@ -11,6 +11,7 @@ BaseSettings, PydanticBaseSettingsSource, SettingsConfigDict, + SettingsError, YamlConfigSettingsSource, ) @@ -105,7 +106,7 @@ def settings_customise_sources( ) -> Tuple[PydanticBaseSettingsSource, ...]: return (YamlConfigSettingsSource(settings_cls),) - with pytest.raises(ValueError): + with pytest.raises(SettingsError, match='Failed to parse settings from .*, expecting an object'): Settings() From c6e6fb9f12292e13b85cca2258489cde9b703207 Mon Sep 17 00:00:00 2001 From: Julian Vanden Broeck Date: Tue, 1 Oct 2024 09:04:17 +0200 Subject: [PATCH 3/3] Raise error if settings from file is not a dict We verify the loaded settings from files is dict, otherwise we raise a SettingsError. With that change, we make Pydantic fails a bit faster. --- pydantic_settings/sources.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pydantic_settings/sources.py b/pydantic_settings/sources.py index 6c43994..01b45d6 100644 --- a/pydantic_settings/sources.py +++ b/pydantic_settings/sources.py @@ -1921,6 +1921,10 @@ def _read_files(self, files: PathType | None) -> dict[str, Any]: settings = self._read_file(file_path) except ValueError as e: raise SettingsError(f'Failed to parse settings from {file_path}, {e}') + if not isinstance(settings, dict): + raise SettingsError( + f'Failed to parse settings from {file_path}, expecting an object (valid dictionnary)' + ) vars.update(settings) return vars