aminghadersohi commented on PR #44096: URL: https://github.com/apache/superset/pull/44096#issuecomment-5612598919
@gabotorresruiz Thanks for the careful review! Addressed the two nits and investigated the channel contract: 1. **Clear predicate:** added `EMPTY_FILTER_SQL_EXPRESSION` in the frontend shared constants and in `superset/constants.py`, with cross-language synchronization comments. Both `getSelectExtraFormData` and data-mask hydration use the frontend constant; `_select_data_mask` uses the backend constant. Expanded the hydration regression test to exercise the actual UI-generated mask as well as the literal permalink wire representation, for both `null` and `[]` cleared values. The frontend producer and backend tool tests retain literal `1 = 0` expectations, so changing either side's emitted predicate independently fails tests rather than silently restoring a saved default. 2. **Callable channel prefix:** once-and-cached is intended for the MCP publisher too; no behavior change made. I checked `TaskManager.init_app` / `get_realtime_channel`, the configuration contract, and the websocket subscription. `REALTIME_CHANNEL_PREFIX` is a deployment namespace, documented as resolved once at startup. The websocket server constructs one `REALTIME_CHANNEL` from its configured prefix and subscribes to that fixed channel. Resolving a context-dependent prefix per MCP call could publish to a different channel with no matching subscriber, breaking that invariant rather than making delivery safer. Strictly, the neutral helper caches per Flask app instance in `app.extensions`, not a global shared across processes; each producer must resolve the same stable deployment prefix as the websocket server. Context-varying prefixes are not supported by this single-channel architecture. Recipient targeting belongs in the envelope's principal/tab routes, not in a dynamically sel ected deployment channel. Existing callable-once and task/non-task channel-sharing tests pass. 3. **Toast callback:** added a one-line comment documenting that the transient, non-persisted toast callback is deliberately non-serializable. **Screenshots remain outstanding.** This environment does not have a running websocket-enabled stack (the local Superset health endpoint is unavailable), so I cannot exercise or capture the live realtime + Undo flow. The unit tests are not a substitute for that live validation, which is still needed before merge. **Local validation:** 43 frontend tests and 50 backend/publisher tests passed. `uvx pre-commit run --files <all PR-changed files>` passed, including mypy and frontend type checking. Changes pushed in c6c5ac499c. CI for the new head has started and is pending; its final result is not yet known. The PR remains in draft. -- 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]
