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

Change BaseSettings defaults #721

Closed
samuelcolvin opened this issue Aug 7, 2019 · 5 comments
Closed

Change BaseSettings defaults #721

samuelcolvin opened this issue Aug 7, 2019 · 5 comments

Comments

@samuelcolvin
Copy link
Owner

@samuelcolvin samuelcolvin commented Aug 7, 2019

So, to do before version 1:

I would like to see these two BaseSettings Config fields default to the following:

class Settings(BaseSettings):

  class Config:
    env_prefix = ""
    case_insensitive = True

Originally posted by @jasonkuhrt in #576 (comment)

@samuelcolvin samuelcolvin added this to the Version 1 milestone Aug 7, 2019
@samuelcolvin

This comment has been minimized.

Copy link
Owner Author

@samuelcolvin samuelcolvin commented Aug 7, 2019

I definitely agree about case_insensitive, not so sure about env_prefix, but I guess you're probably right.

@samuelcolvin samuelcolvin added the Change label Aug 10, 2019
@dmontagu dmontagu mentioned this issue Aug 14, 2019
4 of 4 tasks complete
@samuelcolvin

This comment has been minimized.

Copy link
Owner Author

@samuelcolvin samuelcolvin commented Aug 14, 2019

I've got extremely frustrated with aliases in BaseSettings a number of times:

  • they don't work well with inheritance
  • you very often want to look for more than one alias, eg. here I have redis 'redis_settings': 'REDISCLOUD_URL' so aiohttp-toolbox works "out of the box" (no pun intended) on heroku with the redis cloud plugin I regularly use, but it's a slightly mad default. It would be much better if I could set multiple env varibles to inspect {'REDISCLOUD_URL', 'REDIS_URL'} or whatever.
  • aliases are weird when initialising normally using arguments (eg. for testing) when you don't want to look for env variables, eg. I'd expect to do Settings(pg_dsn='postgres://postgres@localhost:5432/test_db') not have to respect the env var name and do Settings(DATABASE_URL='postgres://postgres@localhost:5432/test_db')

How do we work around this?

I'd therefore like to completely change how environment variables are interpreted in BaseSettings:

from pydantic import BaseSettings

class Settings(BaseSettings):
    pg_dsn: str = 'postgres://postgres@localhost:5432/app'

    class Config:
        fields = {'pg_dsn': {'env': {'DATABASE_URL', 'PG_DSN'}}}

# or using Field (#577):
class Settings(BaseSettings):
    pg_dsn: str = Field('postgres://postgres@localhost:5432/app', env={'DATABASE_URL', 'PG_DSN'})

# creating manually for testing (here i can use the raw field name since alias hasn't changed):
settings = Settings(pg_dsn='postgres://postgres@localhost:5432/testing')

as discussed on #747 env would be completely case insensitive.

thoughts?

@dmontagu

This comment has been minimized.

Copy link
Collaborator

@dmontagu dmontagu commented Aug 14, 2019

It would be much better if I could set multiple env varibles to inspect {'REDISCLOUD_URL', 'REDIS_URL'} or whatever.

+1

In general aliases seem to make more sense to me in the context of BaseModel than BaseSettings. This proposal seems good to me.

@skewty

This comment has been minimized.

Copy link
Contributor

@skewty skewty commented Sep 16, 2019

I personally prefer the 2nd Field method as a developer new to pydantic is more likely to notice and use parameters there. The nested Config from my experience with my team here is less common and comes as a surprise 🎉.

While the usage of set for env is correct in your example, I also think using a list there would be more typical and reduce confusion of it being a dict. Are the additional features of set being used? Was it chosen simply because items should be unique and because order is not important? Perhaps the ordering offered by a list is an important feature to some.

@samuelcolvin

This comment has been minimized.

Copy link
Owner Author

@samuelcolvin samuelcolvin commented Sep 20, 2019

Yes order of env may well be important. Either should work.

@samuelcolvin samuelcolvin mentioned this issue Sep 20, 2019
3 of 5 tasks complete
samuelcolvin added a commit that referenced this issue Oct 1, 2019
@samuelcolvin samuelcolvin mentioned this issue Oct 1, 2019
4 of 4 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.