feat: make environment flag parsing consistent with click
#4443
Conversation
Also add a new utility function: - envflag(): parses environment variables as boolean Applied envflag() to read some environment variables
github-actions
bot
added
area/private-api
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4443 +/- ##
=======================================
Coverage 97.83% 97.83%
=======================================
Files 296 296
Lines 15295 15300 +5
Branches 1713 1714 +1
=======================================
+ Hits 14964 14969 +5
Misses 189 189
Partials 142 142 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
7576457
changed the title
click
|
Documentation preview will be available shortly at https://litestar-org.github.io/litestar-docs-preview/4443 |
litestar/utils/helpers.py
Outdated
|
|
||
| from enum import Enum | ||
| from functools import partial | ||
| from os import getenv |
There was a problem hiding this comment.
| from os import getenv | |
| import os |
litestar/utils/helpers.py
Outdated
| return cast("type[BaseException]", _ExceptionGroup) | ||
|
|
||
|
|
||
| def envflag(varname: str, default: bool = False) -> bool: |
There was a problem hiding this comment.
This does not behave like click. It will consider all values it doesn't consider True to be False, so e.g. a typo like Tru or 2 would also be considered False.
It should raise in that case instead. Please consult the click docs in the original issue I've linked on the actual behaviour
There was a problem hiding this comment.
Thanks for the feedback! I’ve addressed your comments.
According to the click documentation you linked: "Any other value is interpreted as deactivating the flag". This means invalid values like Tru or 2 should return False, not raise an error. Could you clarify what behavior you're expecting?
litestar/utils/helpers.py
Outdated
| envvar = getenv(varname) | ||
| if not envvar: | ||
| return default |
There was a problem hiding this comment.
This will return default if the value is set to "", which is not what we want.
| assert envflag("TEST_FLAG") is True | ||
| del os.environ["TEST_FLAG"] |
There was a problem hiding this comment.
You should use the monkeypatch fixture for this instead.
* envflag: removed default parameter. * envflag: behavior now matches click.BoolParamType. * envflag: raises an error if none of the conditions are met. * tests(envflag): updated tests and applied monkeypatch.
|
@provinzkraut I’m currently using |
2 participants
Description
As mentioned in the issue, handling environment variables is currently inconsistent.
This PR aims to address this by using Click for a more consistent solution:
--quiet-consoleflag to controlLITESTAR_QUIET_CONSOLE.envflag(), which returns True for1,true,t,yes,on,y, and False otherwise.envflag()wherever CLI or other code reads environment variables representing flags.Closes