tien238lnd commented on PR #44573: URL: https://github.com/apache/superset/pull/44573#issuecomment-5948474011
Pushed 087afc80d with the follow-up I promised once the other PR landed, plus @gabotorresruiz's nit. #44337 merged on 29/09, so `update_dataset` is on master now, and its request model reads `model_fields_set` the same way the other three do. `UpdateDatasetRequest` takes `OmittedMeansUnchanged` and `update_dataset` joins `OMITTED_MEANS_UNCHANGED_TOOLS`. Reverting the base reds the check with six fields — `cache_timeout`, `description`, `main_dttm_col`, `sql`, `sync_columns`, `table_name` — so it pins the fix rather than passing by default. Master merged in cleanly at 77fb9914a on the way. The summary now names `create_dataset_metric` among the tools that advertise fewer defaults as a side effect, with the reason, and no longer calls the shared models "chart config models", which stopped being accurate when `DatasetMetricProperties` joined. @aminghadersohi, this and `TableColumnConfig` are the two things your review asked for; your 25/09 note says the originals are addressed, so the only thing holding the PR is the standing `CHANGES_REQUESTED` from before that. Happy to split anything further out. On the `row_limit` follow-up: agreed it belongs in its own change, and I will open it after this merges. A non-null default carries information, so the fix cannot be "drop the default" — it needs the tool to tell an advertised default from a caller's value, which is a different mechanism from this one. One correction to what I have been reporting: the two `test_query_dataset_reexecutes_across_rollover` failures I kept calling pre-existing on master are timezone sensitive, as @gabotorresruiz found. They pass here under `TZ=UTC` and fail under my local ICT, so that is my machine, not master. Everything else: 358 passed in `tests/unit_tests/mcp_service/dataset`, 181 in the registration and inventory suites. -- 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]
