feat(server): optional default for get_setting_value#1669
Conversation
get_setting_value returns an empty string when a key is not found, which forces every call site to remember to treat '' as a sentinel and provide its own fallback. The fallback is sometimes a hard-coded default and sometimes a different code path entirely, leading to inconsistent handling across the codebase. Add an optional argument that defaults to '' (preserves the existing behaviour for every call site) and is returned when the key is not present. New call sites can opt into a more meaningful default without changing the function's signature for existing callers. Refs netalertx#1626.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe ChangesSetting Value Default Parameter
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
thanks for the PR @Arvuno |
Summary
defaultparameter toget_setting_valueinserver/helper.py.""so every existing call site keeps its current behaviour.Why
get_setting_valuecurrently returns""for missing keys, which forces every caller to remember that""is the sentinel and to provide its own fallback. The fallback is sometimes a hard-coded default and sometimes a different code path entirely, leading to inconsistent handling.Adding an optional
defaultis the minimum change that lets call sites clean up their own fallbacks without breaking any of the ~dozens of existing callers.How tested
python -c "from helper import get_setting_value; print(get_setting_value('__nonexistent__'))"→''(unchanged)python -c "from helper import get_setting_value; print(get_setting_value('__nonexistent__', default=42))"→42(new behaviour)ruff/flake8not installed in this environment; the diff is a 4-line signature/docstring change and is type-checked by the project's existingmypyCI.tests/suite (if any) was not run because the project's test runner requires the full Docker stack. The change is verifiable by hand.Notes
Summary by CodeRabbit