villebro commented on code in PR #43689:
URL: https://github.com/apache/superset/pull/43689#discussion_r3888521241
##########
superset-websocket/src/index.ts:
##########
@@ -381,13 +387,15 @@ function isTaskStatusRedisPayload(
const candidate = payload as {
task_id?: unknown;
status?: unknown;
- subscribers?: unknown;
+ channels?: unknown;
};
return (
typeof candidate.task_id === 'string' &&
typeof candidate.status === 'string' &&
- Array.isArray(candidate.subscribers) &&
- candidate.subscribers.every(isTaskStatusSubscriber)
+ Array.isArray(candidate.channels) &&
+ candidate.channels.every(
+ channel => typeof channel === 'string' && channel.length > 0,
+ )
);
Review Comment:
This is intentional and I don't think transitional dual-shape acceptance is
warranted here. The `subscribers`→`channels` payload is an internal Redis wire
contract between `superset/tasks/manager.py` (the sole producer) and this
server (the sole consumer), and both shapes only ever exist *within* this
GAQ→GTF epic — the `task-status` fanout feature is entirely new in the epic and
the epic merges to `master` atomically (umbrella #43407). So there is no
released/prior producer emitting the legacy `{subscribers}` shape in the wild:
a real cluster upgrading from pre-epic has old app pods that don't emit
`task-status` at all (feature absent) and new pods that emit `{channels}`,
which this server reads.
The remaining window — an operator running a *mid-epic* intermediate build
where the app pod and ws pod roll at slightly different times — is documented
in the PR description: deploy the Flask app and ws server together; a mismatch
degrades gracefully to the client's polling backstop (no crash, no leak).
Adding legacy `{subscribers}` acceptance would be dead defensive code (plus the
`principalChannel`-mapping + `isTaskStatusSubscriber` guard I just removed)
guarding against a shape that never ships publicly, so I'd rather keep the wire
contract single-shape. Happy to add it if you'd prefer belt-and-suspenders.
--
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]
