tien238lnd commented on PR #44573:
URL: https://github.com/apache/superset/pull/44573#issuecomment-5806450519
Pushed 93bfb4b25 and corrected the description, after testing the claim it
rested on.
**Which client fills the defaults.** I had written that Claude Code does.
That is wrong as stated: through the CLI over stdio, a call naming two
arguments arrives with exactly those two, before and after this change. Through
Claude's desktop app over a remote MCP connector, the same call reaches the
tool as `input_value={'dataset_id': ..., 'metric': ..., 'metric_name': None,
..., 'extra': None}` and is rejected with `metric_name cannot be empty or
null`. So clients differ, and a server cannot tell a null the caller meant from
a null materialised out of the schema. The description now says that instead.
**Where the null actually lands for charts.** CodeAnt's point about
`update_dashboard` was right, and chasing it found a real gap: `update_chart`
reads `model_fields_set` on the nested chart config, not on the request, so
`UpdateChartRequest` alone left the exposed fields untouched. `BaseChartConfig`
now carries the base, which covers every config in the union.
`update_dashboard` applies each field behind an `is not None` check and ignores
explicit nulls, so it was never affected; it keeps the base so its schema stops
offering a value no branch would honour, and the base docstring no longer
claims null uniformly clears.
**Tests.** `test_omitted_means_unchanged_models_advertise_no_null_default`
walks every subclass of the base, so a model that joins it later is covered
without touching the test. The schema helper now resolves `allOf`-wrapped and
`definitions` refs and fails loudly on a shape it does not recognise, rather
than returning something that trivially passes; a missing tool reports absence
instead of a bare `KeyError`.
Locally: `tests/unit_tests/mcp_service` is green except
`test_query_dataset_reexecutes_across_rollover`, which fails the same way on
master here. The red `babel-extract` is a `messages.pot` drift on master (one
string added, one removed) — nothing in this branch touches a translated string.
--
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]
