feat(listview): realtime list updates baked into useListViewResource - #43436
Conversation
Step 7 of the GAQ→GTF epic — realtime list views, the headline of the new
websocket transport. A list backed by useListViewResource now live-patches its
on-screen rows as the underlying entities change, with no manual refresh.
- Shared realtime client (src/middleware/realtime.ts): owns the single browser
socket and fans the generic {channel, payload} envelope out to any number of
subscribers, so features share one connection. Extracted the socket ownership
out of asyncEvent.ts, which now just subscribes for its tier-2 chart-data
handler. Payload-agnostic and best-effort (reconnect on close).
- useListViewResource gains opt-in realtime (enableRealtime, realtimeIdField):
it subscribes to entity-changes:<resource>, ignores nudges for rows not
currently displayed, debounce-collects the rest (~500ms) and issues ONE
batched fetch of just those rows through the normal authorized list endpoint
(col:<idField> op:in), merging them in place by id. Update-only (no new-row
insertion), so authz/RLS are unchanged and a burst can't hammer the backend.
- Task List is the first surface: realtime enabled with idField 'uuid' (tasks
are UUID-facing); tasks already emit entity-change nudges at completion (6a).
Added 'uuid' to the task API search_columns so the batched refetch can filter.
Tests: shared client (dispatch/subscribe/reconnect), asyncEvent refactored onto
it, useListViewResource live-patch + ignore-offscreen, all green; tsc/eslint/
ruff/pylint clean.
dosubot
Bot
added
change:frontend
|
Bito Automatic Review Skipped - Branch Excluded |
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
/review |
Code Review Agent Run #dba1fdActionable Suggestions - 0Filtered by Review RulesBito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
| // enabled — independent of GLOBAL_ASYNC_QUERIES, since realtime list views | ||
| // (tier-1 entity-change nudges) ride the same socket. Idempotent: a no-op when | ||
| // WEBSOCKET_ENABLED is false, and supersedes any prior socket otherwise. | ||
| connectRealtime(config); |
There was a problem hiding this comment.
Suggestion: When GlobalAsyncQueries is disabled, waitersByTaskId is never initialized because the function returns after this connection call. However, the newly connected socket still delivers realtime:* messages to handleRealtimeMessage, which calls applyStatus and dereferences waitersByTaskId with .get, causing a TypeError for every terminal task message. Initialize the waiter map before the feature-flag return or guard realtime status handling when async queries are disabled. [null pointer]
Severity Level: Minor 🧹
- ⚠️ Realtime chart-task messages generate repeated handler errors.
- ⚠️ Async completion handling cannot process messages in this state.
- ⚠️ Errors are swallowed by the shared realtime dispatcher.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset-frontend/src/middleware/asyncEvent.ts
**Line:** 267:267
**Comment:**
*Null Pointer: When `GlobalAsyncQueries` is disabled, `waitersByTaskId` is never initialized because the function returns after this connection call. However, the newly connected socket still delivers `realtime:*` messages to `handleRealtimeMessage`, which calls `applyStatus` and dereferences `waitersByTaskId` with `.get`, causing a `TypeError` for every terminal task message. Initialize the waiter map before the feature-flag return or guard realtime status handling when async queries are disabled.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fixThere was a problem hiding this comment.
Good catch — fixed in the latest push. waitersByTaskId is now initialized eagerly at declaration (new Map()), so the shared-socket handler is safe when a realtime:* message arrives with GLOBAL_ASYNC_QUERIES off (the socket connects on WEBSOCKET_ENABLED alone): applyStatus finds an empty map and no-ops instead of dereferencing undefined. Added a regression test.
Note this PR also grew a related feature per maintainer request: task rows now live-update on intermediate transitions (IN_PROGRESS/progress/abort), not just completion — nudges are emitted post-commit from the internal status/update/cancel commands, and the nudge now carries the integer id (FAB can't filter a UUIDType column with in, so the batched refetch keys on id).
|
The flagged issue is correct. When To resolve this, you should initialize Here is a concise fix for export const handleRealtimeMessage = (message: RealtimeMessage) => {
if (!waitersByTaskId) return;
const { channel, payload } = message;
if (!channel.startsWith(REALTIME_CHANNEL_PREFIX)) return;
const taskId = payload?.task_id;
const status = payload?.status;
if (typeof taskId === 'string' && typeof status === 'string') {
applyStatus(taskId, status);
}
};Would you like me to fetch and validate the rest of the comments on this PR to see if there are other issues to address? superset-frontend/src/middleware/asyncEvent.ts |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## gaq-to-gtf #43436 +/- ##
==============================================
- Coverage 78.85% 78.84% -0.01%
==============================================
Files 2880 2880
Lines 164476 164476
Branches 38004 38005 +1
==============================================
- Hits 129691 129682 -9
- Misses 32340 32346 +6
- Partials 2445 2448 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
… fix Follow-up on the realtime-listview work: task rows now live-update on every status transition and progress write, not just terminal completion, so the Task List reflects IN_PROGRESS / progress / abort states as they happen. - Emit publish_entity_change post-commit from InternalStatusTransitionCommand (all status transitions incl. PENDING->IN_PROGRESS), InternalUpdateTaskCommand (progress/payload -- already throttled via TASK_PROGRESS_UPDATE_THROTTLE_INTERVAL, so nudge volume is bounded), and CancelTaskCommand (abort). Removed the now redundant nudge from publish_completion (the terminal transition nudges via the command); publish_completion keeps the guaranteed completion signal + tier-2. - The entity-change nudge now carries the integer primary id (via TaskDAO.get_id) instead of the uuid: FAB's default filter converter can't build an "in" filter on a UUIDType column, so the list view's batched refetch must key on the int id. Added "id" (not "uuid") to the task API search_columns; the Task List uses the hook's default "id" idField. - Bump the list-view refetch debounce 500ms->1000ms to coalesce the higher nudge volume into at most one batched fetch per second per list. - Review fix (CodeAnt on #43436): initialize waitersByTaskId eagerly. The shared socket connects whenever WEBSOCKET_ENABLED regardless of GLOBAL_ASYNC_QUERIES, so the subscribed handler can run applyStatus with the flag off and must find a map, not undefined. Added a regression test. Tests: 360 backend task tests, asyncEvent/hooks/realtime/TaskList frontend suites green; mypy/ruff/pylint/tsc clean.
|
Thanks — both bots flagged the same Heads up that this PR also grew a maintainer-requested feature since the initial review: task rows now live-update on intermediate transitions (IN_PROGRESS / progress / abort), not just completion. Nudges are emitted post-commit from the internal status/update/cancel commands, and the nudge now carries the integer |
1 participant
SUMMARY
Step 7 of the GAQ→GTF epic (targets
gaq-to-gtf) — realtime list views, the headline capability of the new websocket transport. A list built onuseListViewResourcenow live-patches its on-screen rows as the underlying entities change, with no manual refresh. The Task List is the first surface; any other list opts in with two args.Shared realtime client (
src/middleware/realtime.ts) — owns the single browser socket and fans the generic{channel, payload}envelope out to any number of subscribers, so features share one connection. Socket ownership is extracted out ofasyncEvent.ts, which now simply subscribes for its tier-2 chart-data handler. The client is payload-agnostic (routes onchannel) and best-effort (reconnects on close); connection auth is thesuperset-ws-tokenJWT cookie riding the handshake.useListViewResourcerealtime (opt-in viaenableRealtime,realtimeIdField) — subscribes toentity-changes:<resource>, ignores nudges for rows not currently displayed, debounce-collects the rest (~500ms) and issues one batched fetch of just those rows through the normal authorized list endpoint (col:<idField> op:in), merging them in place by id. Update-only (no new-row insertion, no full refetch/redraw), so:Task List = first surface — realtime enabled with
realtimeIdField: 'uuid'(tasks are UUID-facing). Tasks already emitentity-changes:tasknudges at completion (shipped in 6a, #43431), so this works end-to-end with no new backend nudge. Addeduuidto the task APIsearch_columnsso the batched refetch can filter by it.Known limitation / follow-up: tasks currently nudge only on terminal completion (per 6a), so intermediate transitions (pending→in_progress, progress) don't yet live-update; emitting nudges on those transitions, and adding nudges at other entities' DAO/command commit points (dashboards/charts/datasets/…), are incremental follow-ups — each new list inherits realtime for free once its backend nudges land.
TESTING INSTRUCTIONS
Automated (all green locally):
jest src/middleware/realtime.test.ts— shared client: dispatch/subscribe/unsubscribe, malformed/handler-error isolation, enabled/disabled connect, reconnect-on-close, disconnect.jest src/middleware/asyncEvent.test.ts— refactored onto the shared client; tier-2 acceleration still settles/rejects/ignores correctly.jest src/views/CRUD/hooks.test.tsx— realtime nudge live-patches a displayed row in place (batchedcol:id op:infetch + merge); nudges for off-screen rows are ignored.pytest tests/unit_tests/tasks/— 359 passed (search_columns addition).tsc/eslint/ruff/pylintclean.Manual (with
WEBSOCKET_ENABLED,DISTRIBUTED_COORDINATION_CONFIG,superset-websocketrunning same-host):ADDITIONAL INFORMATION
WEBSOCKET_ENABLED(realtime transport)