feat: adding json, yaml and toml sources by Smixi · Pull Request #211 · pydantic/pydantic-settings

Smixi · 2024-01-20T04:51:29Z

feat: #210

codecov · 2024-01-20T05:09:45Z

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (cc6dc25) 97.61% compared to head (90a3b4a) 97.63%.
Report is 3 commits behind head on main.

Files	Patch %	Lines
pydantic_settings/sources.py	97.01%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #211      +/-   ##
==========================================
+ Coverage   97.61%   97.63%   +0.01%     
==========================================
  Files           5        5              
  Lines         336      423      +87     
  Branches       71       89      +18     
==========================================
+ Hits          328      413      +85     
- Misses          6        7       +1     
- Partials        2        3       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

samuelcolvin · 2024-01-21T08:40:28Z

I think you mean toml, not JSON - please update the title.

Smixi · 2024-01-21T12:59:15Z

in fact it have the three of them :D.

please review

hramezani · 2024-01-22T10:26:52Z

Thanks @Smixi for the PR 🙏

A couple of things to address:

If the dependency is installed, we need to add the source class by default in settings_customise_sources. So, no need to override the settings_customise_sources to use the new source classes. They should have less priority than the existing sources to be backward-compatible.
CI is broken for Python 3.12. it seems the pyyaml 6.0 can't be installed. Have you tried the pyyaml 6.0.1?
Documentation has to be added in https://github.com/pydantic/pydantic-settings/blob/main/docs/index.md
You've added the JsonConfigSettingsSource as well. not sure we want that. @samuelcolvin what do you think?

hramezani · 2024-01-22T10:42:34Z

 from pydantic_settings.utils import path_type_label

 if TYPE_CHECKING:
+    if sys.version_info.minor >= 11:


Suggested change

if sys.version_info.minor >= 11:

if sys.version_info >= (3, 11):

Smixi · 2024-01-22T20:31:59Z

Thanks @Smixi for the PR 🙏

A couple of things to address:

If the dependency is installed, we need to add the source class by default in settings_customise_sources. So, no need to override the settings_customise_sources to use the new source classes. They should have less priority than the existing sources to be backward-compatible.

CI is broken for Python 3.12. it seems the pyyaml 6.0 can't be installed. Have you tried the pyyaml 6.0.1?

Documentation has to be added in https://github.com/pydantic/pydantic-settings/blob/main/docs/index.md

You've added the JsonConfigSettingsSource as well. not sure we want that. @samuelcolvin what do you think?

If they are default loaded, it means the default settings class is in settings_customise_sources, which will result in the API breaking (people having custom class order will fail). If I don't include it in the settings_customise_sourcesthen it means that the order is not configurable by the user without instantiating yourself the TomlSourceSettings in your order.

hramezani · 2024-01-22T20:52:20Z

Here is the source settings priority right now:

init_settings
env_settings
dotenv_settings
file_secret_settings

So, if we add new sources at the end there shouldn't be a breaking change and there will be new sources that users can use by defining the file path and encoding in config.

It means that releasing the new pydantic-settings won't break existing settings classes and users can benefit from new sources by defining the required configs.

Smixi · 2024-01-22T21:52:06Z

My concerns is more about user having their own custom orders. They will need to reimplement the settings_customise_sources to add the new parameters (like I did in the tests) ?
Pyaml 6.0.1 seems to fix the issue on 3.12.
Still the doc to do.

hramezani · 2024-01-23T09:03:05Z

My concerns is more about user having their own custom orders. They will need to reimplement the settings_customise_sources to add the new parameters (like I did in the tests) ?

That's a good point. didn't think about this.
So, please revert the change and document how to use the new sources. sorry for this

Smixi · 2024-01-23T21:10:14Z

@hramezani That should be good. I just don't know how to deal with the coverage.

hramezani · 2024-01-31T08:55:25Z

+            return {}
+        if isinstance(files, (str, os.PathLike)):
+            files = [files]
+        vars: dict[str, Any | None] = {}


Suggested change

vars: dict[str, Any | None] = {}

vars: dict[str, Any] = {}

hramezani · 2024-01-31T08:55:37Z

+        return vars
+
+    @abstractmethod
+    def _read_file(self, path: Path) -> dict[str, Any | None]:


Suggested change

def _read_file(self, path: Path) -> dict[str, Any | None]:

def _read_file(self, path: Path) -> dict[str, Any]:

hramezani · 2024-01-31T08:55:47Z



+class ConfigFileSourceMixin(ABC):
+    def _read_files(self, files: PathType | None) -> dict[str, Any | None]:


Suggested change

def _read_files(self, files: PathType | None) -> dict[str, Any | None]:

def _read_files(self, files: PathType | None) -> dict[str, Any]:

As in env settings, I think this is allowed in config files to have null values, as it's defined in json and in yaml specs ?
Or is this because None is included in any ?

Changed

hramezani · 2024-01-31T09:06:10Z

+
+    def _read_file(self, file_path: Path) -> dict[str, Any | None]:
+        with open(file_path, encoding=self.yaml_file_encoding) as yaml_file:
+            import_yaml()


Isn't it better to move import_yaml out of the block to check the import before opening the file?

hramezani · 2024-01-31T15:16:51Z

@hramezani That should be good. I just don't know how to deal with the coverage.

I think it can be tested like the old dotenv test in Pydantic V1

and then you can add a new step to the CI to uninstall deps and run the tests to run the above tests

hramezani · 2024-02-01T09:29:01Z

LGTM. Thanks @Smixi.

@samuelcolvin I will keep this PR open. please take a look if you want.

korolkevich · 2024-02-11T12:01:31Z

Hi @Smixi,
Thank you for your work.

I have a question:
If we have nested YAML configuration:
app: pass: pass login: login
How do we replace the default configuration with environment variables, as it is done in Dynaconf (app__pass='new_val' or app.pass='new_val'), and handle deeper configurations? I couldn't find this in your PR.

Smixi · 2024-02-11T17:23:22Z

@korolkevich This PR is only a loader for variables.
I think your use case is more about using this mecanism and declare env_settings before the yaml setting source

samuelcolvin

otherwise LGTM.

Thank you so much.

samuelcolvin · 2024-02-15T18:01:17Z

+foobar = "Hello"
+[nested]
+nested_field = "world!"
+"""


I don't understand what this is?

hramezani · 2024-02-16T09:06:36Z

Thanks @Smixi

Smixi added 3 commits January 20, 2024 05:49

feat: adding json and yaml sources

8d3d0f2

fix: requirement file

ff1b693

fix: requirement file with extra

3c2df89

Smixi added 6 commits January 20, 2024 06:17

fix: error

586a0f9

feat: add toml

e6e79ab

fix: import

0e42bd2

fix: apply kjithin recommandation

9c9c558

fix: issue with typing

0b5109f

test: added test for coverage

a85515f

Smixi marked this pull request as ready for review January 21, 2024 01:24

pydantic-hooky bot added the awaiting author revision label Jan 21, 2024

pydantic-hooky bot assigned Smixi Jan 21, 2024

Smixi changed the title ~~feat: adding json and yaml sources~~ feat: adding json and yaml and toml sources Jan 21, 2024

Smixi changed the title ~~feat: adding json and yaml and toml sources~~ feat: adding json, yaml and toml sources Jan 21, 2024

pydantic-hooky bot added ready for review and removed awaiting author revision labels Jan 21, 2024

pydantic-hooky bot assigned samuelcolvin and unassigned Smixi Jan 21, 2024

hramezani reviewed Jan 22, 2024

View reviewed changes

add settings, documentation and test

d848544

Smixi force-pushed the yaml-json-support branch from ba9fc69 to d848544 Compare January 23, 2024 20:41

lint: uh, ruff is not happy ?

ed406f5

lint: uh, ruff is not happy ?

d7af144

hramezani reviewed Jan 31, 2024

View reviewed changes

change typing for null field

d6d73ee

Smixi added 2 commits January 31, 2024 16:53

add tests for non required deps

4c9317b

fix test on deps

c395cb7

hramezani reviewed Jan 31, 2024

View reviewed changes

Comment thread .github/workflows/ci.yml Outdated

Smixi added 2 commits January 31, 2024 17:04

don't make dotenv optionnal in tests

952e800

dotenv not optionnal, skip test where dep is not fulfilled

da05c31

hramezani approved these changes Feb 1, 2024

View reviewed changes

hramezani mentioned this pull request Feb 5, 2024

Add toml and yaml parsing support to pydantic-settings pydantic/pydantic#8584

Closed

samuelcolvin approved these changes Feb 15, 2024

View reviewed changes

Fix typo in documentation for toml example

90a3b4a

hramezani merged commit 8b92f61 into pydantic:main Feb 16, 2024

hramezani mentioned this pull request Feb 16, 2024

Toml and maybe even yaml support #210

Closed

samuelcolvin mentioned this pull request Feb 17, 2024

We should use tomli not tomlkit #235

Closed

	if sys.version_info.minor >= 11:
	if sys.version_info >= (3, 11):

	def _read_file(self, path: Path) -> dict[str, Any \| None]:
	def _read_file(self, path: Path) -> dict[str, Any]:



		class ConfigFileSourceMixin(ABC):
		def _read_files(self, files: PathType \| None) -> dict[str, Any \| None]:

Uh oh!

Conversation

Smixi commented Jan 20, 2024 • edited by pydantic-hooky bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jan 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

samuelcolvin commented Jan 21, 2024

Uh oh!

Smixi commented Jan 21, 2024

Uh oh!

hramezani commented Jan 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hramezani Jan 22, 2024

Choose a reason for hiding this comment

Uh oh!

Smixi commented Jan 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hramezani commented Jan 22, 2024

Uh oh!

Smixi commented Jan 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hramezani commented Jan 23, 2024

Uh oh!

Smixi commented Jan 23, 2024

Uh oh!

hramezani Jan 31, 2024

Choose a reason for hiding this comment

Uh oh!

hramezani Jan 31, 2024

Choose a reason for hiding this comment

Uh oh!

hramezani Jan 31, 2024

Choose a reason for hiding this comment

Uh oh!

Smixi Jan 31, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hramezani Jan 31, 2024

Choose a reason for hiding this comment

Uh oh!

hramezani commented Jan 31, 2024

Uh oh!

Uh oh!

hramezani commented Feb 1, 2024

Uh oh!

korolkevich commented Feb 11, 2024

Uh oh!

Smixi commented Feb 11, 2024

Uh oh!

samuelcolvin left a comment

Choose a reason for hiding this comment

Uh oh!

samuelcolvin Feb 15, 2024

Choose a reason for hiding this comment

Uh oh!

hramezani commented Feb 16, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Smixi commented Jan 20, 2024 •

edited by pydantic-hooky bot

Loading

codecov bot commented Jan 20, 2024 •

edited

Loading

hramezani commented Jan 22, 2024 •

edited

Loading

Smixi commented Jan 22, 2024 •

edited

Loading

Smixi commented Jan 22, 2024 •

edited

Loading

Smixi Jan 31, 2024 •

edited

Loading