gabotorresruiz commented on PR #44096:
URL: https://github.com/apache/superset/pull/44096#issuecomment-5625735992
Thanks @aminghadersohi, I went through the new commits (`27e8aaef` through
`db84fe48`) and re-ran everything locally at `db84fe4897`. Every item from my
review and the other threads checks out empirically:
- **Shared clear predicate**: exactly what I was hoping for. The new UI-mask
`test.each` cases couple `getSelectExtraFormData` output to hydration
recognition, and the literal `1 = 0` expectations pinned in
`test_apply_dashboard_filters.py` and `filters/utils.test.ts` mean one-sided
drift of either constant now fails tests instead of silently restoring a saved
default.
- **Channel prefix**: your once-per-app rationale makes sense given the
websocket consumer's single fixed subscription, question closed. I ran
`test_channel_resolves_callable_once_under_concurrency` against the pre-lock
publisher three times: deterministic failure there, passes on this head. Good
regression test.
- **Permalink URLs**: the new `APPLICATION_ROOT` and `SCRIPT_NAME` cases
fail against the pre-fix helper and pass here. Since the MCP service boots the
full `create_app()` singleton, `url_for("Superset.dashboard_permalink")`
resolves against the real registered route, so this is safe in the real runtime
too.
- **Malformed permalink hydration**: the pre-guard reducer throws on 5 of
the 8 new malformed-adhoc cases; the guarded one passes all of them.
- **CI budget bump**: I checked recent master push runs and Python-Unit is
being cancelled at ~31 minutes on master itself, so the 45-minute budget is
fixing a live master-wide problem, not papering over a slowdown from this PR.
My local totals match yours: 86 backend/publisher, 41 TaskManager, 51
frontend, all passing. (The two
`test_query_dataset_cached_bounds_across_rollover` failures in my environment
reproduce identically at the base commit, so they are pre-existing and
unrelated, consistent with your note.)
The one thing still standing between this and my approval is the live
realtime + Undo browser validation you flagged yourself. Once you have that run
and the screenshots, I am happy to approve. If getting a websocket-enabled
stack up is the bottleneck, happy to dig in with you.
--
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]
