rusackas opened a new pull request, #45768:
URL: https://github.com/apache/superset/pull/45768
### SUMMARY
The exact pattern `<string>.lower() == "true"` (a strict, case-insensitive
match against the literal word `"true"`, nothing else) was hand-rolled
identically 9 times across production code with no shared helper:
- `superset/extensions/storage/api.py` — 7 sites:
`request.args.get("shared", "false").lower() == "true"`
- `superset/dashboards/api.py` — 1 site: `request.args.get("export_data",
"true").lower() == "true"`
- `superset/db_engine_specs/databend.py` — 1 site, inside
`_encryption_from_tls_parameters`: `secure.lower() == "true"`
`superset/utils/core.py` already has
`cast_to_boolean`/`parse_boolean_string`, but neither is a safe drop-in here:
`parse_boolean_string` additionally treats `"yes"`, `"y"`, `"t"`, `"on"`, `"1"`
as true, which would **silently widen** what each of these 9 call sites accepts
(e.g. `?shared=1` would newly become truthy). That's a behavior change, not a
pure refactor.
This PR adds a new, narrowly-scoped helper, `is_truthy_str()`, placed
alongside `parse_boolean_string` in `superset/utils/core.py`, and adopts it at
all 9 call sites. Its contract is intentionally narrow and matches the old
inline checks exactly:
- `None` -> `default` (default `False`)
- otherwise, strict case-insensitive match against `"true"`, nothing else
As a small simplification enabled by the helper's own `None` handling,
`databend.py`'s `if secure is not None: return secure.lower() == "true"`
collapses to `return is_truthy_str(secure)` (the `sslmode`-set branch above it
is unaffected).
No behavior changes at any of the 9 call sites — this is a pure
de-duplication.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — backend-only, no UI change.
### TESTING INSTRUCTIONS
1. New unit tests for `is_truthy_str()` in
`tests/unit_tests/utils/test_core.py`, covering `None` with both defaults,
`"true"`/`"True"`/`"TRUE"`, and confirming `"1"`/`"yes"`/`"y"`/`"t"`/`"on"` are
**not** treated as true (pinning the "no widening vs. `parse_boolean_string`"
contract):
```
pytest tests/unit_tests/utils/test_core.py -k "truthy or boolean" -q
```
2. Existing tests for the touched call sites pass unchanged:
```
pytest tests/unit_tests/extensions/storage/ -q
pytest tests/unit_tests/db_engine_specs/test_databend.py -q
pytest tests/unit_tests/utils/test_core.py -q
```
3. `pre-commit run` on the staged diff (ruff, mypy, pylint, etc.) passes
clean.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
