villebro opened a new pull request, #43895:
URL: https://github.com/apache/superset/pull/43895

   ### SUMMARY
   
   Addresses the blocking finding in the umbrella review ([#43407 
review](https://github.com/apache/superset/pull/43407#pullrequestreview-5117266775)):
 the per-dashboard async override could not be saved.
   
   The Properties modal writes the override into `json_metadata.async_mode`, 
but `DashboardJSONMetadataSchema` did not declare the field. 
`validate_json_metadata` calls `DashboardJSONMetadataSchema().validate(..., 
partial=False)`, which rejects unknown fields, so the dashboard `PUT` failed 
with `422 "Unknown field"` and the override never persisted.
   
   This declares `async_mode` as an optional string validated to `default` / 
`force_on` / `force_off` (the values the frontend `resolveAsyncMode` chain 
emits; the modal deletes the key for `default`). No backend execution change — 
chart-data async is driven by a **request-level** `async_mode` flag on 
`/chart/data`, not this metadata key, which is frontend-consumed.
   
   ### TESTING INSTRUCTIONS
   
   Folded the persistence assertion into the existing 
`test_update_dashboard_persists_metadata_fields_without_dedicated_handling` 
(integration `dashboards/api_tests.py`): the PUT payload now includes 
`async_mode: "force_off"` and asserts it round-trips. Schema accept/reject was 
also verified directly (`force_on`/`force_off`/`default` valid; unknown value 
rejected).
   
   ### ADDITIONAL INFORMATION
   
   - [ ] Has associated issue:
   - [x] Required feature flags: `GLOBAL_ASYNC_QUERIES` (the override only 
surfaces in the UI when async is available)
   - [ ] Changes UI
   - [ ] Includes DB Migration
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   
   ---
   
   Note on the review's second (non-blocking) point — the possibly-leaked GTF 
abort listener flaking the oauth2 test's caplog: I could not reproduce or 
localize it. The tasks tests that start a real abort listener (`test_handlers`, 
`test_timeout`) already stop it in fixture teardown, and the others 
(`test_manager`, `test_query_cancel`) mock the listener, so there's no obvious 
unstopped listener in the current code. Suggest handling as a separate 
follow-up once we have the failing CI seed to reproduce deterministically, 
rather than adding a speculative teardown here.


-- 
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]

Reply via email to