mikebridge commented on code in PR #44028:
URL: https://github.com/apache/superset/pull/44028#discussion_r4007172222
##########
superset/daos/dashboard.py:
##########
@@ -343,9 +473,9 @@ def set_dash_metadata(
if (positions := data.get("positions")) is not None:
# find slices in the position data
slice_ids = [
- value.get("meta", {}).get("chartId")
+ chart_id
for value in positions.values()
- if isinstance(value, dict)
+ if (chart_id := _layout_chart_id(value)) is not None
Review Comment:
Fixed in 7e4c47b9ee — integral floats (`123.0`) and digit strings (`"123"`)
are now coerced to the int in `_layout_chart_id`, so a live chart in either
form is recognised and kept, while an absent id in those forms is still
repaired; fractional floats, non-digit strings, and bools stay `None`. A
parametrized unit test covers every form, and a reconcile test pins the
live-kept / absent-repaired split. Good catch on the orphan-tile mechanics
(excluded from the rebuild AND unrepairable) — that is exactly why coercion
rather than repair was the right fix.
##########
superset/commands/dashboard/update.py:
##########
@@ -77,11 +77,18 @@ def run(self) -> Model:
ObjectType.dashboard, self._model.id, self._model.tags,
tags
)
- # Re-serialize position_json to escape 4-byte Unicode characters
+ # Re-serialize position_json to escape 4-byte Unicode characters,
+ # and reconcile it against membership: a layout node referencing a
+ # chart that no longer resolves to any Slice row (hard-deleted) is
+ # swapped for a placeholder so ``position_json`` cannot keep
+ # accumulating dangling chart references (sc-115325). The
+ # ``set_dash_metadata`` path reconciles its own ``positions`` from
+ # ``json_metadata``; this covers a PUT that sends only the raw
+ # ``position_json`` field.
if position_json := self._properties.get("position_json"):
- self._properties["position_json"] = json.dumps(
- json.loads(position_json)
- )
+ positions: object = json.loads(position_json)
+ reconcile_position_json(positions, self._model.id)
Review Comment:
Fixed in 7e4c47b9ee — the raw-field reconcile now runs only when
`json_metadata` does not carry `positions`; when it does, `set_dash_metadata`
reconciles and writes those and the raw field is superseded, so the dead work
(including its membership query) is skipped. The precedence is recorded in the
comment, `json_metadata` is parsed once and handed to `set_dash_metadata`, and
an integration test pins both the precedence and that the raw reconcile is not
invoked on a PUT sending both.
--
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]
