Repository navigation
fix(parameters): ignore malformed POWERTOOLS_PARAMETERS_MAX_AGE instead of crashing - #8757
Conversation
…ad of crashing An empty or non-integer POWERTOOLS_PARAMETERS_MAX_AGE crashed every parameters, secrets and AppConfig lookup with a raw ValueError from int() before any AWS call was made (13 call sites share one resolver). Resolve the variable through a helper that falls back to the documented default with a PowertoolsUserWarning naming the ignored value (ADR 0006: warn now, harden in a future major), mirroring the aws-powertools#8734 approach for POWERTOOLS_IDEMPOTENCY_DISABLED. Recognized integer values, including 0 (no caching), behave exactly as before. Regression test fails on develop with the ValueError and passes with the fix for both malformed values.
|
Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need. |
powertools-for-aws-oss-automation
Bot
added
the
size/L
Manishram-ai
left a comment
Manishram-ai
left a comment
There was a problem hiding this comment.
Thanks for this. I called nine public entry points (get_parameter, get_parameters, get_parameters_by_name, SSMProvider.get, SSMProvider.get_parameters_by_name, get_secret, get_secrets_by_name, SecretsProvider.get and get_app_config) with the providers' fetch methods stubbed, and POWERTOOLS_PARAMETERS_MAX_AGE set to "" and then "abc". On develop all nine raise ValueError: invalid literal for int() with base 10: ''. On this branch they all return, each one emits a single PowertoolsUserWarning, and the warning points at my calling line every time, so stacklevel=3 works for both the module functions and the provider methods. tests/functional/parameters, tests/unit/test_shared_functions.py and tests/functional/feature_flags pass (454 tests), and the new test fails on develop with the original ValueError.
Separate from this PR, the same config-drift case exists for POWERTOOLS_PARAMETERS_SSM_DECRYPT. With it set to "", the five SSM entry points above raise ValueError: invalid truth value '' through resolve_truthy_env_var_choice, on develop and on this branch. Could be a good follow-up issue next to this one and #8728.
| except ValueError: | ||
| warnings.warn( | ||
| f"Ignoring invalid {constants.PARAMETERS_MAX_AGE_ENV} value {env!r}; " | ||
| f"falling back to the default of {DEFAULT_MAX_AGE_SECS} seconds.", |
There was a problem hiding this comment.
The docstring above says this hardens in a future major. If that's the plan, ADR 0006 asks for the warning itself to say what will change and when, so the message could mention that it becomes an error in the next major. If the fallback is meant to stay, the "harden in a future major" part of the docstring could go instead.
leandrodamascena
left a comment
leandrodamascena
left a comment
There was a problem hiding this comment.
Thanks @AP24110011695! Ran the whole parameters suite on top of today's develop, all green, and the twelve call sites now go through one place. Good to see an explicit max_age still wins. Goes out in 3.41.0, no need to rebase, you'll be in the release notes.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #8757 +/- ##
========================================
Coverage 97.18% 97.18%
========================================
Files 323 323
Lines 16555 16562 +7
Branches 1496 1497 +1
========================================
+ Hits 16089 16096 +7
Misses 326 326
Partials 140 140 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Awesome work, congrats on your first merged pull request and thank you for helping improve everyone's experience! |
|
Awesome work, congrats on your first merged pull request and thank you for helping improve everyone's experience! |
Issue number: closes #8747
Summary
Changes
resolve_max_age()is a rawint(env), so an empty or non-integerPOWERTOOLS_PARAMETERS_MAX_AGEraisedValueErrorfrom every Parameters/Secrets/AppConfig entry point (12 call sites) before any AWS call was made.resolve_parameters_max_age()inutilities/parameters/base.py: an explicitchoicealways wins; otherwise the environment variable is parsed, and aValueErroremits aPowertoolsUserWarningnaming the ignored value and falls back to the documented default (DEFAULT_MAX_AGE_SECS= 300s), per ADR 0006 (warn now, harden in a future major).resolve_max_age, andos/constants/DEFAULT_MAX_AGE_SECSwhere no longer used) removed.PowertoolsUserWarningper ADR 0006 rather than a debug log — happy to align with fix(idempotency): treat empty or unparsable POWERTOOLS_IDEMPOTENCY_DISABLED as not disabled #8734's debug-log style if you prefer the two byte-identical.0(no caching) — behave exactly as before.User experience
Before:
POWERTOOLS_PARAMETERS_MAX_AGE=""(a common config-drift case) madeget_parameter(),get_secret(),SSMProvider().get()and every other lookup raiseValueError: invalid literal for int() with base 10: ''until the environment was fixed.After: the same calls succeed with the documented default caching age, and one warning names the ignored value so the typo is visible.
Verification
test_ssm_provider_get_with_invalid_max_age_env_var(tests/functional/parameters/_boto3/test_utilities_parameters.py): ondevelop(bc3136c) it fails with the reportedValueError; with this change it passes for both""and"abc", asserting the warning, the returned value, and that the SDK call still happens.uv run pytest tests/functional/parameters tests/unit/test_shared_functions.py: 263 passed.uv run pytest tests/functional/feature_flags: 191 passed.uv run pytest tests/unit: 626 passed; the 8 failures are pre-existing on cleandevelopon this machine (missing optional extras aws_xray_sdk etc.) and unrelated to this change.uv run mypy aws_lambda_powertools/utilities/parameters/: clean;ruff format+ruff checkclean on all touched files.developbefore filing Bug: POWERTOOLS_PARAMETERS_MAX_AGE set to an empty or non-integer value crashes every parameters call #8747 (empty and non-integer crash; unset baseline reaches the SDK).This PR was prepared with AI assistance; the repro, tests and every run above were executed and verified by me against
develop(bc3136c) as described.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.