pydantic / pydantic-settings

Settings management using pydantic
https://docs.pydantic.dev/latest/usage/pydantic_settings/
MIT License
508 stars 50 forks source link

add `PyprojectTomlConfigSettingsSource` #255

Closed ITProKyle closed 3 months ago

ITProKyle commented 4 months ago

PR template copied from https://github.com/pydantic/pydantic/blob/18d39febf00cb07954e19ebbdbd8168ef20e3df1/.github/PULL_REQUEST_TEMPLATE.md as this repo does not contain one.

Change Summary

Related issue number

resolves #253

Checklist

hramezani commented 4 months ago

Thanks @ITProKyle for this PR. I still haven't looked at it in detail.

added SettingsConfigDict.pyproject_toml_depth (only used by PyprojectTomlConfigSettingsSource) specify a number of levels up the directory tree to look for a pyproject.toml if not found in the current working directory if not provided, 0 is used if no file is found, path in current working directory is used

Isn't it better to have a pyproject_toml_file config to specify the path of pyproject.toml file. the default value could be the pyproject.toml. I think the pyproject_toml_depth config is not needed as we can use the pyproject_toml_file

added SettingsConfigDict.toml_table_header (only used by PyprojectTomlConfigSettingsSource) used to optionally select a table to fill values from if not provided the root table is used header was used over path (mention in the feature request) after reviewing the terminology used in TOML v1.0.0

Let's have a default header. something like tools.pydantic-settings

ITProKyle commented 4 months ago

I changed toml_table_header to pyproject_toml_table_header for consistency of scoped configs. This would also make it possible to add support for toml_table_header to the TOML source if desired for loading global settings (e.g. from ~/.conf/pydantic-settings/config.toml) that does not need to follow the same nested table structure.

pyproject_toml_table_header defaults to using the [tool.pydantic-settings] table.


Isn't it better to have a pyproject_toml_file config to specify the path of pyproject.toml file. the default value could be the pyproject.toml. I think the pyproject_toml_depth config is not needed as we can use the pyproject_toml_file

The rationale behind supporting discovery is to mirror the behavior of most tools (I have used) that use pyproject.toml for configuration - accept an explicit path and try to find the file if that path is not provided. I am fine with adding a pyproject_toml_file config field if you feel that it would be beneficial but removal of discovery slightly hinders the advantages of using a standardized file.

However, I feel that pyproject_toml_file would have limited use in pydantic-settings current state because, as mentioned in #250, it can't be set at runtime which would be the primary case for setting that config field. I exposed this as in arg when instantiating the source class so that functionality can be implemented in the future or by a user of pydantic-settings if they want to sort out providing an explicit path.

Here are a few references for the discovery behavior:

black and ruff's discovery behavior can both be checked within this project by cd pydantic_settings before running them.

hramezani commented 4 months ago

I changed toml_table_header to pyproject_toml_table_header for consistency of scoped configs. This would also make it possible to add support for toml_table_header to the TOML source if desired for loading global settings (e.g. from ~/.conf/pydantic-settings/config.toml) that does not need to follow the same nested table structure.

pyproject_toml_table_header defaults to using the [tool.pydantic-settings] table.

👍

The rationale behind supporting discovery is to mirror the behavior of most tools (I have used) that use pyproject.toml for configuration - accept an explicit path and try to find the file if that path is not provided.

Can we have the same behavior here as well?(I still haven't checked the code. So, if you implemented like this it would be fine)

ITProKyle commented 4 months ago

Can we have the same behavior here as well?(I still haven't checked the code. So, if you implemented like this it would be fine)

ya, that's there. explicitly provided when instantiating the source class (PyprojectTomlConfigSettingsSource(settings_cls, toml_file=Path(...)) which disables any attempt at discovery. if not provided (or None), discovery is used from the current working directory up int number of directories in the tree (SettingsConfigDict(pyproject_toml_depth=<int>)) until found. if not found within int directories, the current working directory is chosen to populate the object's attribute even though the file does not exist just to have something to fall back to.

By default, discovery is set to a depth of 0 (e.g. no discovery, only use the current working directory) making it opt-in in case someone doesn't want this behavior.

hramezani commented 4 months ago

Thanks @ITProKyle for updating the PR. Overall LGTM. I left a couple of comments.

Please update

hramezani commented 3 months ago

Thanks @ITProKyle