tien238lnd commented on PR #44573:
URL: https://github.com/apache/superset/pull/44573#issuecomment-5827409068
Pushed f147b2a44, which addresses both points of @aminghadersohi's review
and the two notes from @gabotorresruiz.
The substantive one was real and mine: `TableColumnConfig` is merged per
column label with `exclude_unset=True`, so `update_chart` still wiped a stored
`columnWidth` or `d3NumberFormat` when a client filled the defaults — and both
of my checks stayed green because they read only the top-level `properties`.
The check now walks the whole schema at every depth, which covers a tool's
inlined parameters and the `$defs` of `model_json_schema()` alike, and it only
reaches zero with `TableColumnConfig`, `MetricCurrency`, `ColumnRef`,
`AxisConfig` and `FilterConfig` on the base as well. Reverting any one of the
five reds it.
The red `pre-commit` was the mypy hook reading `pytest.fail` as `Any`, so
the helper looked like it fell through; both exits are `raise AssertionError`
now, which also narrows `tool` in the guard.
The summary names the three extra tools and `get_chart_type_schema`, whose
advertised schemas change because the chart config models are shared. Bito's
CWE-409 note on the loop is left alone — Amin is right that deleting a key from
each inner field dict never resizes `properties`.
Locally: 3497 passed, 3 skipped across
`tests/unit_tests/mcp_service/{chart,dashboard,dataset,utils}` and
`test_mcp_tool_registration.py`;
`test_query_dataset_reexecutes_across_rollover` fails the same way on the merge
base here. mypy reports nothing in the two files I touched.
--
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]
