villebro commented on PR #43696:
URL: https://github.com/apache/superset/pull/43696#issuecomment-5471303814
Thanks for the thorough re-review — pushed `0754fc0` addressing the
follow-ups.
**1. Cancel now uses the exact submit-time tab id.** Rather than thread the
id through every producer, the server echoes the tab id it recorded
(`get_request_tab_id()`) in the 202, and the client cancels with
`asyncJob.tab_id` (falling back to `getTabId()` only for a job without one).
This is the value the backend actually stored as the consumer, so it closes the
POST→202 reassignment window authoritatively. (Same root issue CodeAnt flagged
on `asyncEvent.ts:330`.)
**2. Cancel tab-id validation.** The cancel endpoint now reads the tab id
through the same `get_request_tab_id()` validator as submit (silent-drop of
malformed/oversized values, consistent behavior), and the schema field
documents the `^[A-Za-z0-9_-]{1,64}$` constraint.
**3. Invalid policy routes no longer fall back to principal-grain.**
`publish_task_status` now distinguishes *policy returned `None`* (→
principal-grain) from *policy scoped delivery but every key was rejected* (→
publish nothing; the interval poll is the correctness path). Broadening to all
of a principal's tabs would have broken the policy's intended isolation. Test
updated to lock in the new behavior.
**4. Keepalive boundary + a race.** Bumped the client refresh to 0.6 of the
token lifetime so timer jitter can't land exactly on the 0.5 sliding-window
boundary and miss the re-mint. Also fixed a race CodeAnt caught on
`realtime.ts`: the refresh's `.finally` now bails if the socket it belonged to
was already replaced by a reconnect (`socket !== ws`), so it can't tear down a
newer healthy socket.
**5. Public/anonymous.** Confirmed out of scope for this work —
anonymous/Public-role sockets are intentionally not offered; the docs state the
broadcast is authenticated-global. No code change.
**6. Stale comments.** Dropped the leftover `realtime:` / `entity-changes:*`
wording from `manager.py`, `async_queries.py`, and `channel.py` so they match
the single-channel `{topic, scope, routes, payload}` model.
Re scaling (raised separately): the single `realtime` channel is fine behind
N pods — every pod `SUBSCRIBE`s and receives every message, then delivers only
to sockets it holds, which is exactly what lets targeted delivery work without
sticky sessions or a connection directory. It's the same global-fan-out
property the prior two-channel design had; the per-principal-channel
`SUBSCRIBE`/`UNSUBSCRIBE` sharding is the documented escape hatch if a very
large fleet ever makes the firehose parse cost matter. And a transient WS
outage is a non-event: the cursor poll keeps completing charts and reconciling,
the socket reconnects on its own.
All affected unit tests pass (Python tasks/websocket/chart-data; frontend
realtime/asyncEvent). The frontend type-check hook still reports the
pre-existing `gaq-to-gtf` failures unrelated to this PR.
--
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]
