pydantic-settings icon indicating copy to clipboard operation
pydantic-settings copied to clipboard

Release 2.2 breaks enviroment prefix

Open dleifeld opened this issue 1 year ago • 15 comments
trafficstars

This .envs file:

TESTING=TRUE

CONNECTION_ADDRESS="example.com"
CONNECTION_PORT=443

worked in Pydantic Settings 2.1 with this python code without problems:


from pydantic_settings import BaseSettings, SettingsConfigDict

class ConnectionSettings(BaseSettings):
    model_config = SettingsConfigDict(env_prefix='connection_', env_file='.envs')

    address: str
    port: int


def get_conn_params() -> ConnectionSettings:
    settings = ConnectionSettings()   # pyright: ignore
    print(settings)

    return settings

# if executed as a script
if __name__ == "__main__":
    get_conn_params()

But in Pydantic Settings 2.2 this error is thrown:

pydantic_core._pydantic_core.ValidationError: 1 validation error for ConnectionSettings
testing
  Extra inputs are not permitted [type=extra_forbidden, input_value='TRUE', input_type=str]

In my opinion the setting env_prefix should imply, that inputs without this prefix are ignored. I am not sure why else a prefix should be used.

dleifeld avatar Feb 21 '24 14:02 dleifeld

Thanks @dleifeld for reporting this.

Actually, it is not a breaking change. it was a bug that got fixed in 2.2

You can fix your problem by ignoring the extra by adding extra='ignore' to config

hramezani avatar Feb 21 '24 19:02 hramezani

I didn't know that env_prefix is only an alias, which means CONNECTION_PORT and PORT treated as the same.

In the documentation this behavior is mentioned only at the very end. I find this behavior surprising and would suggest to mention it earlier in the docs.

dleifeld avatar Feb 22 '24 08:02 dleifeld

Would you like to make a PR and improve the docs?

hramezani avatar Feb 22 '24 08:02 hramezani

This was a breaking change for us too. We have a project with internal components that take different settings. In the code there are different pydantic-settings classes, but we keep all the env vars in a single .env file. I appreciate the workaround is easy to implement but disagree about it not being a breaking change.

benjsec avatar Feb 22 '24 12:02 benjsec

Thanks @benjsec for your opinion here.

I am sorry that the new release breaks your code.

The change that is introduced in 2.2 was a fix for these two issues: https://github.com/pydantic/pydantic-settings/issues/219 and https://github.com/pydantic/pydantic-settings/issues/220. which I think was a bug.

The bug was there from the beginning. we had two options:

  1. Continue with the bug in V2 and release the fix in V3
  2. Or, Fix it now and release it in V2.2.

As, there was a way(adding extra='ignore') to fix the codes, I decided to release it in V2.2

BTW, I really appreciate your feedbacks @benjsec and @dleifeld. This will help us in improving the library

hramezani avatar Feb 22 '24 12:02 hramezani

Thank you for your helpful comments and the work on pydantic!

dleifeld avatar Feb 22 '24 12:02 dleifeld

@hramezani - I just wanted to say that I do understand the comments above but I'd like to flag two things:

  • This was a breaking change for me too (and even if the previous behaviours were a bug given the documentation) - this has led to me debugging something for a while to get to this issue - I'd suggest if we are using semver for pydantic-settings (as we seem to be) then this would be a major release update, as it is a breaking change (even as a bugfix) or if the kind of changes shown here is going to be the release policy for pydantic-settings it would be good to know so as library users we can pin to a minor release).
  • I'm not sure the current (and documented) behaviour is actually desired - yes it can be fixed by extra='allow', but this now means that e.g. passing in extras in python init will not throw a ValidationError unlike before. The previous behaviour was more logical to common development patterns - using .env files for all environment variables for a given environment level (dev/test etc.) - could be fixed by an env_file_extra variable or similar

djpugh avatar Feb 23 '24 21:02 djpugh

Sorry @djpugh to hear that the change breaks your code and I am agree that this could be release in V3. but there are some major change that we think about it for V3 and those are not ready for V3. That's why I decided to release the fix soon.

I'm not sure the current (and documented) behaviour is actually desired

As I mentioned before it was a fix for reported issues and I think this is right. I would suggest using one .env file per model. regarding having env_file_extra config I would like to keep the number of configs less because of maitainace effort. for this special config, I think it makes it more confusing because we already have the extra config.

Thanks @djpugh for your valuable feedback it helps us a lot.

hramezani avatar Feb 25 '24 12:02 hramezani

@hramezani - thanks for the reply, it seems that we should expect changes that may require downstream code changes for minor releases of pydantic-settings. I guess I'll be making sure to pin the minor versions then.

Using one dotenv file per model feels like a big imposition on how people might expect to use dotenv files more widely (at least in my experience with them), and it feels counter-intuitive to how the EnvSettingsSource logic works, I guess I'll implement a workaround for my library's usage of pydantic-settings.

djpugh avatar Feb 25 '24 17:02 djpugh

@djpugh no, this is not the plan to break user codes and i will take care of this more from now on.

hramezani avatar Feb 25 '24 17:02 hramezani

I thought i put all environment variables into one .env. How else I am supposed to work with them?

dleifeld avatar Feb 27 '24 12:02 dleifeld

I thought i put all environment variables into one .env. How else I am supposed to work with them?

Probably you can split your .env file into multiple files and use them in your settings models. Or, you can have one global .env file and use it in all settings model but you need to set extra='ignore' config in you settings models

hramezani avatar Feb 27 '24 12:02 hramezani

@dleifeld in addition to the two options @hramezani mentioned, a third option that allows keeping everything in a single .env file without compromising validation with extra='ignore' is to add a "GlobalSettings" model as shown in this comment. That's what I've ended up using since the suggestion to change the pydantic-settings behavior to validate only vars that start with env_prefix was turned down.

gsakkis avatar Feb 28 '24 00:02 gsakkis

Came across this issue after 1/2 day of search... I agree with most that the current behavior is worse than illogical/misleading : it is not useful. The main purpose of pydantic is avoiding silly mistakes, like, wrong name in config, missing config, spotting quickly something that has been renamed etc.

Proposition

Since adding another config would be overkill and add maintenance costs, here what I suggest :

Adding a new value for extra, which would be something like forbid_from_env_when_prefixed which would restore the previous behavior, i.e. raise an error only if the extra field has the prefix.

Rationals

The suggested idea to split the .env is not viable : no other framework REQUIRE to do that, and I have never seen split .env in 8 years of career.

The BaseSettings merge is not a good solution either, because the .env may also contain variables for other part of the project that could even not be in python. Here again, splitting may lead to duplication, since some variables could also be shared.

Another benefit from this could be to catch extra env variables (not from the .env) with the prefix.

I'm not sure what implication this proposition has on the code base, but if this idea is retained, I may also provide a PR. Tell me what you think ?

hl037 avatar May 24 '24 09:05 hl037

Thanks @hl037 for your suggestion. but adding a new extra option to pydantic-settings does not work because extra config belongs to pydantic. also, adding these options to Pydantic won't work because Pydantic doesn't care about env.

hramezani avatar May 24 '24 09:05 hramezani

I am going to close this issue. we will release pydantic-settings 2.3 soon. we will do our best to not break your code again.

Sorry for that

hramezani avatar Jun 03 '24 12:06 hramezani