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 e348a6b6..94d268f0 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 29186018..124520e8 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 fd25de67..d17f32cb 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 919f8366..6c439948 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 94d268f0..7a6ba5d3 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 124520e8..0d2f70ed 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 d17f32cb..10f5fd11 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 6c439948..01b45d69 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