Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More __init__ Overwrite Support #346

Open
acederberg opened this issue Jul 19, 2024 · 2 comments
Open

More __init__ Overwrite Support #346

acederberg opened this issue Jul 19, 2024 · 2 comments
Assignees
Labels

Comments

@acederberg
Copy link

acederberg commented Jul 19, 2024

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
Copy link
Member

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 added a commit to acederberg/pydantic-settings-yaml that referenced this issue Jul 23, 2024
Made an issue on the ``pydantic/pydantic_settings`` github to see if I
could make this work the way I'd like. See  pydantic/pydantic-settings#346.
@acederberg
Copy link
Author

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.

@hramezani hramezani added V3 and removed unconfirmed labels Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants