rusackas opened a new pull request, #45763:
URL: https://github.com/apache/superset/pull/45763
### SUMMARY
`superset/utils/cache_manager.py` carried its own copy of the algorithm-name
to hashlib-constructor mapping, flagged by its own comment: *"Hash function
lookup table matching superset.utils.hashing"*. This PR removes that
duplication.
**Important nuance (why this isn't a one-line import swap):**
`cache_manager.py`
and `hashing.py` each defined a dict with the same literal keys (`"sha256"`,
`"md5"`), but they are **not interchangeable** — they have different return
shapes:
- `hashing.py`'s `_HASH_FUNCTIONS` returns a **finalized hex digest string**
(`lambda data: hashlib.sha256(data).hexdigest()`), used by
`hash_from_str`/`hash_from_dict`.
- `cache_manager.py`'s `_HASH_METHODS` returned the **raw hashlib
constructor** itself (`hashlib.sha256`), because it's used as
Flask-Caching's
`hash_method`, which must behave like `hashlib.md5`/`hashlib.sha256`
directly — a callable that, given bytes, returns a hash **object**
supporting `.update()`/`.hexdigest()`/`.digest()`, not a string.
So the real duplication was narrower than "these two dicts are the same" —
it was specifically the `{"sha256": hashlib.sha256, "md5": hashlib.md5}`
raw-constructor mapping that both files independently defined.
**The fix:**
1. `hashing.py` now defines that raw-constructor mapping once, as
`_HASH_CONSTRUCTORS`, and derives `_HASH_FUNCTIONS` from it (via a dict
comprehension that avoids the classic late-binding closure-in-a-loop bug).
2. `cache_manager.py` deletes its own `_HASH_METHODS` dict and imports
`_HASH_CONSTRUCTORS` from `hashing.py` instead.
3. `ConfigurableHashMethod.__call__` now calls `hashing.py`'s existing
`get_hash_algorithm()` helper instead of re-reading
`current_app.config["HASH_ALGORITHM"]` directly — `get_hash_algorithm()`
already does exactly that lookup.
This is a pure dedup with **no behavior change**: `ConfigurableHashMethod`
still returns a hash object (not a string) and still raises the same
`ValueError` for an unsupported algorithm.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A (internal refactor, no UI change)
### TESTING INSTRUCTIONS
- Existing unit tests cover this directly:
- `tests/unit_tests/utils/test_cache_manager.py` (`ConfigurableHashMethod`,
`SupersetCache`) — updated 3 tests that patched
`superset.utils.cache_manager.current_app` to instead patch
`superset.utils.cache_manager.get_hash_algorithm`, since the
implementation now calls that helper instead of reading
`current_app.config` directly. All 29 tests pass.
- `tests/unit_tests/utils/test_hashing.py` (`hash_from_str`,
`hash_from_dict`, `get_hash_algorithm`) — all pass unmodified.
- Also ran `tests/unit_tests/tasks/test_utils.py`,
`tests/unit_tests/queries/query_object_test.py`, and
`tests/unit_tests/utils/screenshot_test.py`, which also exercise
`hash_from_str`/`hash_from_dict` indirectly — all 174 pass.
- Manual sanity check: `configurable_hash_method(b"test")` still returns an
object (not a string) whose `.hexdigest()` matches
`hashlib.sha256(b"test").hexdigest()` under the default algorithm, and
matches `hashlib.md5(b"test").hexdigest()` when the algorithm is `"md5"`;
an unsupported algorithm still raises `ValueError`.
- `pre-commit run` (staged files only, including `mypy`) 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]
