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

python-dotenv should be optional #149

Closed
maxzhenzhera opened this issue Aug 23, 2023 · 3 comments
Closed

python-dotenv should be optional #149

maxzhenzhera opened this issue Aug 23, 2023 · 3 comments

Comments

@maxzhenzhera
Copy link

maxzhenzhera commented Aug 23, 2023

From pyproject.toml:

requires-python = '>=3.7'
dependencies = [
    'pydantic>=2.0.1',
    'python-dotenv>=0.21.0',
]
dynamic = ['version']

python-dotenv is always installed.

From pydantic_settings.sources:

def read_env_file(
file_path: Path, *, encoding: str | None = None, case_sensitive: bool = False
) -> Mapping[str, str | None]:
try:
from dotenv import dotenv_values
except ImportError as e:
raise ImportError('python-dotenv is not installed, run `pip install pydantic[dotenv]`') from e
file_vars: dict[str, str | None] = dotenv_values(file_path, encoding=encoding or 'utf8')
if not case_sensitive:
return {k.lower(): v for k, v in file_vars.items()}
else:
return file_vars

  • here is the logic of the optional python-dotenv
  • and the wrong message in exception to install as pydantic extra (which already does not have this extra)

One more place where the project depends on python-dotenv is tests.test_settings:

try:
import dotenv
except ImportError:
dotenv = None

But even here checks are made to skip tests if python-dotenv is not installed (one of the tests with such a check):
@pytest.mark.skipif(not dotenv, reason='python-dotenv not installed')
def test_env_file_config(env, tmp_path):
p = tmp_path / '.env'
p.write_text(test_env_file)
class Settings(BaseSettings):
a: str
b: str
c: str
model_config = SettingsConfigDict(env_file=p)
env.set('A', 'overridden var')
s = Settings()
assert s.a == 'overridden var'
assert s.b == 'better string'
assert s.c == 'best string'

While making PR I understood the reason for making it in tests.
Finally, I changed the CI workflow.

Selected Assignee: @dmontagu

@samuelcolvin
Copy link
Member

Why not keep python-dotenv as required dependency and remove the error handling?

I mean it was different before where pydantic was used for many applications which didn't require use of Settings.

If there's a good reason to make it optional, I'd be happy to consider it - but my first reaction is that the requirement should not be optional and we just change the code that assumes it is.

@maxzhenzhera
Copy link
Author

Personally, I do not use .env files.
So, I never load it using pydantic-settings and therefore code that depends on python-dotenv is never executed.

Yep, obviously, I can not say that python-dotenv is kind of heavy dependency and it bothers me.
Just in my case, it is unused and without it, my dependencies will become a little bit cleaner.

@hramezani
Copy link
Member

related code for optional dotenv removed at 0226fe2

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.

4 participants