rusackas opened a new pull request, #45761:
URL: https://github.com/apache/superset/pull/45761
### SUMMARY
`JsonKeyValueCodec` (in `superset/key_value/types.py`) imported the stdlib
`json` module directly. Encoding any payload containing a `datetime` (or
`Decimal`/numpy value) raised `TypeError: Object of type datetime is not JSON
serializable`, which the codec's own `except TypeError` wraps into
`KeyValueCodecEncodeException` — i.e. the codec crashed on exactly the input
class it was presumably written to guard against.
`superset.utils.json` already solves this: it wraps `simplejson` with a
`default=json_iso_dttm_ser` encoder built specifically to serialize
`datetime`/`date`/`Decimal`/numpy values (and more, via `base_json_conv`) to
JSON-compatible forms.
This PR switches the import to `from superset.utils import json`, the same
shadowing convention already used in `superset/forms.py`,
`superset/sql_lab.py`, `superset/jinja_context.py`, and
`superset/datasource/api.py`. The `json.dumps(value)` / `json.loads(value)`
call sites are unchanged in source — they now simply resolve to the project's
implementation. `MarshmallowKeyValueCodec` inherits the fix since it subclasses
`JsonKeyValueCodec` and calls `super().encode`/`super().decode`.
This codec backs real call sites that can carry datetime-valued payloads,
including dashboard/explore/SQL Lab filter-state
(`superset/temporary_cache/api.py`),
`superset/dashboards/excel_export/download_link.py`,
`superset/commands/distributed_lock/base.py`, `superset/daos/report.py`,
`superset/commands/database/oauth2.py`, `superset/db_engine_specs/base.py`,
`superset/key_value/shared_entries.py`, and
`superset/semantic_layers/execution.py`.
I also checked the rest of `superset/key_value/` for other direct
stdlib-`json` usages that duplicate this codec's logic;
`superset/key_value/utils.py` already uses
`superset.utils.json.json_dumps_w_dates` correctly, so no other file needed a
change.
### BEFORE/AFTER
**Before:**
```python
>>> JsonKeyValueCodec().encode({"foo": datetime(2024, 1, 1)})
KeyValueCodecEncodeException: Object of type datetime is not JSON
serializable
```
**After:**
```python
>>> JsonKeyValueCodec().encode({"foo": datetime(2024, 1, 1)})
b'{"foo": "2024-01-01T00:00:00"}'
```
### TESTING INSTRUCTIONS
- `pytest tests/unit_tests/key_value/codec_test.py -q`
- Added a regression case to `test_json_codec` that round-trips a dict
containing a `datetime` value through `encode`/`decode` and asserts it does not
raise (it would have raised `KeyValueCodecEncodeException` before this fix).
- Updated the existing bare-`set` case in the same parametrized test:
encoding a bare `set` no longer raises, because `superset.utils.json`'s
`base_json_conv` already converts sets to lists instead of erroring the way
stdlib `json` does. This is pre-existing, intentional behavior of
`superset.utils.json`, not something new introduced here, and no call site of
this codec relies on sets being rejected.
- Also ran the broader surrounding suites that exercise this codec
(`tests/unit_tests/key_value/`, `tests/unit_tests/dao/key_value_test.py`,
`tests/unit_tests/distributed_lock/`, `tests/unit_tests/semantic_layers/`,
`tests/unit_tests/extensions/metastore_cache_test.py`,
`tests/unit_tests/extensions/storage/test_codecs.py`,
`tests/unit_tests/reports/config_dao_test.py`) — all pass (175 tests).
- `pre-commit run` on the staged diff passes (mypy, ruff, pylint, etc.).
### 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
**Follow-up noted, not fixed here:** `JsonKeyValueCodec.decode` only catches
`TypeError`, but `superset.utils.json.loads` raises
`simplejson.JSONDecodeError` (a `ValueError` subclass) on malformed JSON, not
`TypeError` — so a genuinely corrupt stored payload wouldn't be wrapped in
`KeyValueCodecDecodeException`. This was already true before this change
(stdlib `json.loads` has the same `JSONDecodeError`-not-`TypeError` behavior),
so it's pre-existing and intentionally left out of scope for this fix.
🤖 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]
