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.6.0 breaks environment variable loading #445

Open
palfrey opened this issue Oct 18, 2024 · 10 comments
Open

2.6.0 breaks environment variable loading #445

palfrey opened this issue Oct 18, 2024 · 10 comments
Assignees

Comments

@palfrey
Copy link

palfrey commented Oct 18, 2024

The following code works with 2.5.2, but not 2.6.0

from typing import TypeVar
from pytest import MonkeyPatch
from pydantic import BaseModel

from pydantic import BaseModel
from pydantic_settings import BaseSettings as PydanticBaseSettings

_Model = TypeVar("_Model", bound=BaseModel)


class BaseSettings(PydanticBaseSettings):
    @classmethod
    def load(cls: type[_Model]) -> _Model:
        return cls.model_validate({})

class _TestSettings(BaseSettings):
    x: int

def test_base_settings_with_env_variables(monkeypatch: MonkeyPatch) -> None:
    x = 1
    monkeypatch.setenv("X", str(x))
    assert _TestSettings.load() == _TestSettings(x=x)

I've gone through all the changes between the versions, but I'm not seeing anything obvious that's broken this?

@hramezani
Copy link
Member

Thanks @palfrey for reporting this. I will check it

@hramezani
Copy link
Member

@palfrey I've investigated the problem and it comes from a change from me.

you are using the model_validate in your code. model_validate accepts obj but you are passing a {}.

Probably the old current behavior was wrong and the current one is right.

can't you change your code to

class BaseSettings(PydanticBaseSettings):
    @classmethod
    def load(cls: type[_Model]) -> _Model:
        return cls()

BTW, we can revert my change but I would like to keep it if you can fix the problem on your side.

Please let me know.
Thanks!

@palfrey
Copy link
Author

palfrey commented Oct 18, 2024

@palfrey I've investigated the problem and it comes from a change from me.

you are using the model_validate in your code. model_validate accepts obj but you are passing a {}.

Probably the old current behavior was wrong and the current one is right.

We can change on our side, that's fine, especially as I've just done some testing and the revised code is fine on both 2.5.2 and 2.6.0. I'm not quite sure where we got the model_validate bit from, that's been copy/pasted a bunch around our codebase, but I'm not seeing that usage of it in any of the docs.

Thanks for investigating!

@rseeley
Copy link

rseeley commented Oct 18, 2024

Myself and my team use the model_validate pattern because using normal class instantiation results in typing errors from pylance ("Argument missing for parameter "SOME_ENVAR""). Is there a recommended way around that issue without using pyright: ignore comments? If not, I like the load function method here and will probably use that.

@hramezani
Copy link
Member

There is a mypy plugin for pydantic that solves the problem for mypy. Unfortunately there isn't anything for pyright

@rseeley
Copy link

rseeley commented Oct 18, 2024

No worries, we'll go the load route. Thanks!

@jessemyers-lettuce
Copy link

We use model_validate() extensively for similar reasons: we frequently want to load objects -- especially settings -- from partial state, type checkers can't reason about which attributes will be loaded automatically and which won't, and we don't want to have type ignore markers everywhere.

@hramezani
Copy link
Member

@jessemyers-lettuce @rseeley @palle-k

I will discuss with the team about reverting the change that made model_validate not work and will back to you.

@hramezani hramezani reopened this Oct 18, 2024
@jessemyers-lettuce
Copy link

Thanks, I'd also be perfectly happy with another mechanism that achieves partial construction without running afoul of type checkers.

@hramezani
Copy link
Member

hramezani commented Oct 22, 2024

I discussed this with the team and we decided to find a way to make model_validate({}) and sub-models inheriting from BaseSettings work again.

For now please stay on 2.5.2.

Thanks for your feedback!

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

No branches or pull requests

5 participants