pydantic / pydantic-settings

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

want to leak non-`env_prefix`'d values from dotenv? #265

Closed mknaw closed 5 months ago

mknaw commented 6 months ago

Hi,

I'd like to inquire about the intentionality of this dotenv loading behavior when env_prefix is set. Namely, even with an env_prefix set in the model config, it looks like we happily read values whose name matches the field names even when they are not prefixed:

def test_with_prefix_with_dotenv(tmp_path):
    p = tmp_path / '.env'
    p.write_text('apple=incorrect')

    class Settings(BaseSettings):
        apple: str

        model_config = SettingsConfigDict(env_file=p, env_prefix='foobar_')

    s = Settings()
    assert s.apple == 'correct'

I see per the documentation:

Pydantic settings loads all the values from dotenv file and passes it to the model, regardless of the model's env_prefix. So if you provide extra values in a dotenv file, whether they start with env_prefix or not, a ValidationError will be raised.

Respectfully, reading such unprefixed value seems like a footgun to me (imagine e.g. DEBUG vs SOME_OTHER_THING__DEBUG), especially since the env-sourced variables do not exhibit this behavior. Intuitively, I'd expect overall similar behavior for both sources.

But, even if this load-all behavior is desired, should there at least be an effort to prefer the dotenv value whose prefix matches over that with no prefix? Without having looked at the actual parsing code, it appears the last entry currently wins, which can be seen by modifying the previous test:

...
    p.write_text('foobar_apple=correct')
    p.write_text('apple=incorrect')
...

Thanks in advance.

hramezani commented 6 months ago

Thanks @mknaw for reporting this.

It is because Dotenv source parses extra values(apple=incorrect in your case) at the end and it will override the old value(foobar_apple=correct).

I think we can change it to don't override the value but I am afraid it will break some user code that depends on this.