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

2.2.0 broke support for nested BaseSettings with base type (e.g. using ABC) #241

Closed
moonrail opened this issue Feb 19, 2024 · 5 comments · May be fixed by #244
Closed

2.2.0 broke support for nested BaseSettings with base type (e.g. using ABC) #241

moonrail opened this issue Feb 19, 2024 · 5 comments · May be fixed by #244
Assignees

Comments

@moonrail
Copy link

Hello altogether,

change #204 is a breaking change. It introduces a dict-cast that leads to nested copy-by-value for BaseSettings instances. In 2.1.0 it was copy-by-reference.

See this line: https://github.com/pydantic/pydantic-settings/pull/204/files#diff-b4b68ace3cb0cf2820d1d882735748b675a379c39463ccc89700a9618a80a2a2R124

This breaks any use-case with a base class/type and/or abc classes, where the type is not explicitly known beforehand.
E.g. for pluggable authentication:

from abc import ABC, abstractmethod
from pydantic import SecretStr, HttpUrl
from pydantic_settings import BaseSettings
from typing import Type


class BaseAuth(ABC, BaseSettings):
    @property
    @abstractmethod
    def token(self) -> str:
        """returns authentication token for XYZ"""
        pass


class CustomAuth(BaseAuth):
    url: HttpUrl
    username: str
    password: SecretStr

    _token: SecretStr = None

    @property
    def token(self):
        ...  # (re)fetch token
        return self._token.get_secret_value()


class Settings(BaseSettings):
    auth: Type[BaseAuth]


s = Settings(
    auth=CustomAuth(
        url='https://127.0.0.1',
        username='some-username',
        password='some-password'
    )
)
print(s.auth)
print(type(s.auth))

Upon execution you'll receive following ValidationException:

Traceback (most recent call last):
  File "/tmp/test_pydantic_settings_nesting.py", line 32, in <module>
    s = Settings(
        ^^^^^^^^^
  File "/tmp/venv/lib/python3.11/site-packages/pydantic_settings/main.py", line 85, in __init__
    super().__init__(
  File "/tmp/venv/lib/python3.11/site-packages/pydantic/main.py", line 171, in __init__
    self.__pydantic_validator__.validate_python(data, self_instance=self)
pydantic_core._pydantic_core.ValidationError: 1 validation error for Settings
auth
  Input should be a subclass of BaseAuth [type=is_subclass_of, input_value={'url': Url('https://127....SecretStr('**********')}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.6/v/is_subclass_of

From this POV 2.2.0 should be 3.0.0, as this is a Breaking Change. Moving from copy-by-reference to copy-by-value with casting is a change that can have a lot of impact in an interfacing-focused library.

As a side note: The amount of breaking changes in Minor-Releases across the pydantic project increased in the last year. We have more and more debugging sessions regarding it and are scratching our heads, if and how we are possibly "holding it wrong" or its something out of our control and pydantic is not as stable as it should be...

@hramezani
Copy link
Member

Thanks @moonrail for reporting this and sorry for the problem.

This breaks any use-case with a base class/type and/or abc classes, where the type is not explicitly known beforehand.

agree

From this POV 2.2.0 should be 3.0.0, as this is a Breaking Change. Moving from copy-by-reference to copy-by-value with casting is a change that can have a lot of impact in an interfacing-focused library.

Yes, you are right. As this wasn't the plan I am going to introduce a config flag for this change and make this disable by default. So, You will have the old behavior after my fix.

As a side note: The amount of breaking changes in Minor-Releases across the pydantic project increased in the last year. We have more and more debugging sessions regarding it and are scratching our heads, if and how we are possibly "holding it wrong" or its something out of our control and pydantic is not as stable as it should be...

We really try to not introduce breaking changes in minor releases but we are not aware of all the use-cases.
Thanks for helping us by reporting issues.

@hramezani
Copy link
Member

hramezani commented Feb 19, 2024

@moonrail
The example you've provided is not working on pydantic-settings==2.1.0

Could you please test it with pydantic-settings==2.1.0?

Could you provide pydantic, pydantic-core and pydantic-settings versions of the setup that your example is working on it?

@hramezani
Copy link
Member

I think the validation error here is correct because you defined auth: Type[BaseAuth] but provided value is an instance of CustomAuth. So, probably your problem will be fixed by changing auth: Type[BaseAuth] to auth: BaseAuth

@moonrail
Copy link
Author

Hey @hramezani
you're right, I've copy-pasted one of our internal code usages (that one is quite old and incorrectly uses Union[Type[XyzClass], Any]) and mended it until it looked postable, but did not test it again against 2.1.0.
Sorry about that - did not intend to cause more work than necessary.

Tested code is:

from abc import ABC, abstractmethod
from pydantic import SecretStr, HttpUrl
from pydantic_settings import BaseSettings


class BaseAuth(ABC, BaseSettings):
    @property
    @abstractmethod
    def token(self) -> str:
        """returns authentication token for XYZ"""
        pass


class CustomAuth(BaseAuth):
    url: HttpUrl
    username: str
    password: SecretStr

    _token: SecretStr = None

    @property
    def token(self):
        ...  # (re)fetch token
        return self._token.get_secret_value()


class Settings(BaseSettings):
    auth: BaseAuth


s = Settings(
    auth=CustomAuth(
        url='https://127.0.0.1',
        username='some-username',
        password='some-password'
    )
)
print(s.auth)
print(type(s.auth))

pydantic-settings 2.0.0 and 2.1.0 print:

url=Url('https://127.0.0.1/') username='some-username' password=SecretStr('**********')
<class 'main.CustomAuth'>

While pydantic-settings 2.2.0 prints:

Traceback (most recent call last):
  File "/tmp/test_pydantic_settings_nesting.py", line 32, in <module>
    s = Settings(
        ^^^^^^^^^
  File "/tmp/venv/lib/python3.11/site-packages/pydantic_settings/main.py", line 85, in __init__
    super().__init__(
  File "/tmp/venv/lib/python3.11/site-packages/pydantic/main.py", line 171, in __init__
    self.__pydantic_validator__.validate_python(data, self_instance=self)
TypeError: Can't instantiate abstract class BaseAuth with abstract method token

@hramezani
Copy link
Member

pydantic-settings 2.2.1 just released. I reverted that change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants