pydantic / pydantic-settings

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

More ``__init__`` Overwrite Support #346

Open acederberg opened 1 month ago

acederberg commented 1 month ago

Hello Everybody!

My apologies if this is a bit long.

The problem

Support for passing additional overwriting keyword arguments for additional sources to __init__ is confusing. I would like to overwrite some values in one of my sources, but it looks like I'd have to create my source in the settings_customize_sources classmethod to get it into _settings_build_values, which means I'd have to overwrite the _settings_build_values method somehow. This is less than desirable, since I have no means to inject those sources using super.

My Use Case

I have a bit of an odd use case for a library I'm writing. This library supports loading YAML from many sources optionally with reloading of the files or not when creating new instances. I know that there is a YAML source defined here, but my use case is different enough that I want to write my own source.

Essentially, I want to inject sources on an instance level when overwrites (_yaml_files and _yaml_settings) are provided, otherwise it uses the source created with the subclass hook:

class BaseYamlSettings(BaseSettings):
    __yaml_settings_cls__: ClassVar[CreateYamlSettings]
    __yaml_settings_self__: CreateYamlSettings | None
    __yaml_exclude__: bool

    if TYPE_CHECKING:
        model_config: ClassVar[loader.YamlSettingsConfigDict]

    def __init_subclass__(cls, **kwargs):
        """Create the default ``CreateYamlSettings`` instance."""
        super().__init_subclass__(**kwargs)
        cls.__yaml_settings_cls__ = CreateYamlSettings(cls)

    def __init__(
        self,
        _yaml_files: loader.YamlSettingsFilesInput | None = None,
        _yaml_reload: bool | None = None,
        _yaml_exclude: bool = False,
        **kwargs,
    ):
        # NOTE: If any overwrites are added, then do not use ``__yaml_settings_cls__``.
        #       This is a pain because the other option is to overwrite
        #       ``_settings_build_values``, which likely results in having
        #       to overwrite ``settings_customise_sources``.
        if _yaml_files is not None:
            yaml_settings = self.model_config.copy()
            yaml_settings.update(
                yaml_files=_yaml_files,
                yaml_reload=_yaml_reload,
            )
            self.__yaml_settings_self__ = CreateYamlSettings(self.__class__)
        else:
            self.__yaml_settings_self__ = self.__yaml_settings_cls__

        self.__yaml_exclude__ = _yaml_exclude
        super().__init__(**super()._settings_build_values(kwargs))

The main problem is that _settings_build_values uses the source customization hook settings_customize_sources which is a classmethod - there is no means to pass instance data to the hook.

I don't want to copy and paste the definition of _settings_build_values from here and maintain it in my library, that is not an elegant or easy to maintain solution. I also want to keep all of the functionality of BaseSettings without overwriting signatures ideally. I would like to do something like

def _settings_build_values(self, **kwargs):
    kwargs["sources_extra"] = [self.__yaml_settings_self__]
    super()._settings_build_values(self, **kwargs)

and have sources_extra concatenation to the local sources.

Is there an obvious way to do this I am not noticing? Should I just give up and not use BaseSettings and write an equivalent? If not, should I make a pull request doing roughly that which is specified in the block of code here?

hramezani commented 1 month ago

Hello @acederberg, Thanks for reporting this issue.

Support for passing additional overwriting keyword arguments for additional sources to init is confusing. I would like to overwrite some values in one of my sources, but it looks like I'd have to create my source in the settings_customize_sources classmethod to get it into _settings_build_values, which means I'd have to overwrite the _settings_build_values method somehow. This is less than desirable, since I have no means to inject those sources using super.

I agree, it is confusing and you have to do a lot of work which is not good. unfortunately, we can't change it in V2, because it will introduce a breaking change but we will improve it in V3 for sure to make it clear and easy.

Is there an obvious way to do this I am not noticing? Should I just give up and not use BaseSettings and write an equivalent? If not, should I make a pull request doing roughly that which is specified in the block of code here?

No, there is no clear way to do this. any improvement PR is welcome but it shouldn't introduce a new breaking change

acederberg commented 1 month ago

Hi @hramezani!

Changes should be in #350. I went with a callback pattern, as it would appear to be the least intrusive. I do not think it has any breaking changes.