villebro commented on code in PR #43843:
URL: https://github.com/apache/superset/pull/43843#discussion_r3935256618
##########
superset/config.py:
##########
@@ -3015,6 +3015,15 @@ def EMAIL_HEADER_MUTATOR( # pylint:
disable=invalid-name,unused-argument # noq
WEBSOCKET_JWT_COOKIE_DOMAIN = None
WEBSOCKET_JWT_EXPIRATION_SECONDS = int(timedelta(minutes=15).total_seconds())
+# Prefix for the realtime pub/sub channel (default ""). Redis pub/sub is not
+# scoped by DB number, so deployments sharing one Redis/Valkey would
cross-deliver
+# realtime envelopes (opaque entity-change + task-status nudges) —
cross-tenant id
+# leakage and spurious refetches. Set a per-deployment value (e.g.
"<keyPrefix>:")
+# here AND on the websocket server (REALTIME_CHANNEL_PREFIX env) to isolate
them.
+# May be a string or a zero-argument callable, resolved once at startup,
mirroring
+# Superset's other cache-key helpers. Empty is a no-op for single-instance
setups.
+REALTIME_CHANNEL_PREFIX: Callable[[], str] | str = ""
Review Comment:
Good catch — fixed in d764350. The websocket README
(`superset-websocket/README.md`) no longer calls the channel fixed: the
"Superset Configuration" section now documents that the subscribed channel is
`<REALTIME_CHANNEL_PREFIX>realtime`, that the prefix (`REALTIME_CHANNEL_PREFIX`
in Superset, `REALTIME_CHANNEL_PREFIX`/`realtimeChannelPrefix` on the server)
must be set identically on both sides, and that the Redis ACL must allow
subscribing to the **resulting** channel (e.g. `tenant-a:realtime`, not just
`realtime`). Added a row to the coordination table and a pointer from the
channel-envelope section.
##########
tests/unit_tests/tasks/test_manager.py:
##########
@@ -99,6 +113,13 @@ def test_get_completion_channel_custom_prefix(self):
== "custom:complete:test-uuid"
)
+ def test_get_realtime_channel(self):
+ assert TaskManager.get_realtime_channel() == "realtime"
+
+ def test_get_realtime_channel_custom_prefix(self):
+ TaskManager._realtime_channel_prefix = "tenant-a:"
Review Comment:
Agreed — the helper test alone wouldn't catch a revert to the literal
`realtime`. Added `test_publishes_on_configured_prefixed_channel` (in
`TestTaskManagerEntityChange`, d764350) that configures
`REALTIME_CHANNEL_PREFIX` via `init_app` and invokes the public
`publish_entity_change`, asserting `CoordinationService.publish` receives
`tenant-a:realtime`. This exercises the full init_app → public publisher →
`_publish_realtime` wiring, so a regression to the literal channel now fails a
test.
--
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]
