Repository navigation
fix(shared): treat an empty boolean env var as unset - #8782
zhanfan-818 wants to merge 1 commit into
Conversation
`os.getenv(name, default)` only falls back to `default` when the variable is
absent. When it is present but empty -- e.g. `POWERTOOLS_DEV=` in a
docker-compose file, a CI job, or `export POWERTOOLS_DEV=$UNSET_VAR` -- it
returns `""`, which `strtobool()` rejects:
>>> os.environ["POWERTOOLS_DEV"] = ""
>>> Logger()
ValueError: invalid truth value ''
The same one-line pattern appears at seven call sites across four modules, so
this fixes them together instead of special-casing POWERTOOLS_DEV:
- shared/functions.py: powertools_dev_is_set, powertools_debug_is_set
- metrics/functions.py: is_metrics_disabled (POWERTOOLS_DEV, METRICS_DISABLED)
- utilities/circuit_breaker: CIRCUIT_BREAKER_DISABLED
- utilities/idempotency: IDEMPOTENCY_DISABLED (both call sites)
Using `os.getenv(name) or default` keeps the strict behaviour for genuinely
invalid values (`POWERTOOLS_DEV=treu` still raises) while letting an empty
value mean "not configured", which is what the default was for.
Tests: regression coverage for the two shared resolvers and for
is_metrics_disabled; all of them fail on the previous code and pass here.
The circuit breaker and idempotency sites are the same one-line change but
need a persistence store to exercise -- happy to add those tests if wanted.
Relates-to: aws-powertools#8765
powertools-for-aws-oss-automation
Bot
added
the
size/M
|
|
Closing this as asked -- @krishbuilds01 has the issue, and the scope in your comment is narrower than what this branch does (it touches the two Two things worth handing over rather than throwing away, in case they save time:
And the process point is fair, no argument from me: I should have commented on the issue before opening a PR instead of alongside it. That is on me, and I will do it that way next time. @krishbuilds01 -- good luck with it, and if you want a second pair of eyes on the resolver when it is up, ping me. |
Issue number: closes #8765
Summary
Changes
os.getenv(name, default)only falls back todefaultwhen the variable is absent. When it is present but empty --POWERTOOLS_DEV=in a docker-compose file, a CI job, orexport POWERTOOLS_DEV=$UNSET_VAR-- it returns"", andstrtobool()rejects that withValueError: invalid truth value ''.The same one-line pattern is at seven call sites across four modules, so this fixes them together instead of special-casing
POWERTOOLS_DEV:shared/functions.py:124powertools_dev_is_setPOWERTOOLS_DEVshared/functions.py:136powertools_debug_is_setPOWERTOOLS_DEBUGmetrics/functions.py:158is_metrics_disabledPOWERTOOLS_DEVmetrics/functions.py:159is_metrics_disabledPOWERTOOLS_METRICS_DISABLEDutilities/circuit_breaker/circuit_breaker.py:96circuit_breakerPOWERTOOLS_CIRCUIT_BREAKER_DISABLEDutilities/idempotency/idempotency.py:109_idempotentPOWERTOOLS_IDEMPOTENCY_DISABLEDutilities/idempotency/idempotency.py:219idempotent_functionPOWERTOOLS_IDEMPOTENCY_DISABLEDThe change is
os.getenv(name, default)toos.getenv(name) or default, seven times. Strict validation is preserved:POWERTOOLS_DEV=treustill raisesValueError. Only the empty case changes, to mean "not configured" -- which is what the default was there for.Regression tests were added for the two shared resolvers and for
is_metrics_disabled.User experience
Before, an empty value was a hard crash at import or construction time, with a message that does not say which variable is at fault:
After, the same code behaves as if the variable were not set: dev mode is off, no warning is emitted, and
Logger()initialises normally.Verification
Reproduced on
developat111f93b, before the change:Confirmed the new tests fail without the fix and pass with it:
The
circuit_breakerandidempotencysites get the identical one-line change but need a persistence store to exercise; I did not want to bolt a heavyweight harness onto this PR. Happy to add those tests if you want them here rather than in a follow-up.AI use: the change and its tests were drafted with an AI coding assistant. I reviewed every line, reproduced the failure locally on
develop, and ran the suite myself -- the output above is from my machine, not generated.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.