sadpandajoe commented on code in PR #43237:
URL: https://github.com/apache/superset/pull/43237#discussion_r3872393910


##########
superset/ai/eventbus.py:
##########
@@ -0,0 +1,314 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""
+Carries streamed events from whatever produced them to the HTTP response.
+
+Two implementations, matching the two execution modes. Inline execution needs
+nothing more than an in-process queue. Worker execution needs a shared,
+*replayable* channel — replayable because a browser that loses its connection
+must be able to rejoin a run already in progress, which rules out
+publish/subscribe: a subscriber that was absent when an event was published
+never sees it.
+
+The Redis implementation therefore uses streams, and reuses the cache backend
+that Superset's async-query channel already configures rather than introducing
+a second Redis client to operate.
+"""
+
+from __future__ import annotations
+
+import logging
+import queue
+from abc import ABC, abstractmethod
+from collections.abc import Iterator
+from typing import Any
+
+from superset.ai.events import StreamEvent
+from superset.ai.types import StreamEventType
+from superset.utils import json
+
+logger = logging.getLogger(__name__)
+
+#: Yielded by :meth:`BaseEventBus.consume` when nothing arrived within the poll
+#: interval, so a caller can emit a keep-alive rather than block indefinitely.
+IDLE = None
+
+#: Terminal event types. Seeing one ends consumption, so a reader does not hang
+#: waiting for a producer that has already finished.
+_TERMINAL = frozenset(
+    {StreamEventType.DONE, StreamEventType.ERROR, StreamEventType.CANCELLED}
+)
+
+
+class BaseEventBus(ABC):
+    """A per-run channel of events."""
+
+    @abstractmethod
+    def publish(self, run_id: str, event: StreamEvent) -> None:
+        """Append an event to a run's channel."""
+
+    @abstractmethod
+    def consume(
+        self,
+        run_id: str,
+        timeout_seconds: float,
+        poll_seconds: float = 1.0,
+    ) -> Iterator[StreamEvent | None]:
+        """
+        Yield a run's events until a terminal one arrives or time runs out.
+
+        Yields :data:`IDLE` when a poll interval passes with nothing new, which
+        is the caller's cue to send a keep-alive frame.
+        """
+
+    @abstractmethod
+    def close(self, run_id: str) -> None:
+        """Release any resources held for a run."""
+
+
+class MemoryEventBus(BaseEventBus):
+    """
+    An in-process queue per run.
+
+    Correct only when the producer and the streaming request share a process.
+    Selecting this alongside worker execution would leave every stream silent,
+    which :func:`get_event_bus` refuses to allow.
+    """
+
+    def __init__(self) -> None:
+        self._queues: dict[str, queue.SimpleQueue[StreamEvent]] = {}
+
+    def _queue_for(self, run_id: str) -> queue.SimpleQueue[StreamEvent]:
+        return self._queues.setdefault(run_id, queue.SimpleQueue())
+
+    def publish(self, run_id: str, event: StreamEvent) -> None:
+        self._queue_for(run_id).put(event)
+
+    def consume(
+        self,
+        run_id: str,
+        timeout_seconds: float,
+        poll_seconds: float = 1.0,
+    ) -> Iterator[StreamEvent | None]:
+        import time
+
+        # Deliberately not ``_queue_for``: reading must not create a channel.
+        # This bus lives for the life of the process, so a client polling
+        # unknown run identifiers would otherwise grow the dict without bound.
+        channel = self._queues.get(run_id)
+        deadline = time.monotonic() + timeout_seconds
+
+        while True:
+            remaining = deadline - time.monotonic()
+            if remaining <= 0:
+                return
+            if channel is None:
+                # The producer may not have published yet; look again rather
+                # than deciding the run does not exist. Only report idle if it
+                # is still absent, so a channel that appeared during the wait
+                # is drained on this pass instead of costing an extra tick.
+                channel = self._queues.get(run_id)
+                if channel is None:
+                    yield IDLE
+                    time.sleep(min(poll_seconds, remaining))
+                continue
+            try:
+                # Bounded by whichever is sooner, so a generous poll interval
+                # cannot overshoot the caller's deadline.
+                event = channel.get(timeout=min(poll_seconds, remaining))
+            except queue.Empty:
+                yield IDLE
+                continue
+            yield event
+            if event.type in _TERMINAL:
+                return
+
+    def close(self, run_id: str) -> None:
+        self._queues.pop(run_id, None)
+
+
+class RedisStreamEventBus(BaseEventBus):
+    """
+    A Redis stream per run.
+
+    Replayable by construction: a reconnecting reader starts from the beginning
+    of the stream and catches up, which is what makes worker execution usable
+    from a browser on a flaky connection.
+    """
+
+    def __init__(
+        self,
+        cache: Any,
+        prefix: str = "ai-events-",
+        ttl_seconds: int = 900,
+    ) -> None:
+        self._cache = cache
+        self._prefix = prefix
+        self._ttl = ttl_seconds
+
+    def _stream(self, run_id: str) -> str:
+        return f"{self._prefix}{run_id}"
+
+    def publish(self, run_id: str, event: StreamEvent) -> None:
+        payload = {
+            "data": json.dumps({"type": event.type.value, "payload": 
event.payload})
+        }
+        # A failure to publish must not kill the run that is producing useful
+        # work; the reader will time out and the answer is still persisted.
+        try:
+            self._cache.xadd(self._stream(run_id), payload, "*", 10_000)

Review Comment:
   In worker mode, a run whose browser never opens the stream never reaches 
`close()`, so this `xadd` creates a Redis key with no expiration and abandoned 
runs accumulate indefinitely. Could the producer set the configured expiry once 
the stream reaches its terminal event rather than relying on a consumer to 
clean it up?



##########
docs/admin_docs/configuration/ai-assistant.mdx:
##########
@@ -0,0 +1,489 @@
+---
+title: AI Assistant
+hide_title: true
+sidebar_position: 17
+version: 1
+---
+
+# AI Assistant
+
+The AI Assistant is a conversational interface for exploring your data. A user
+asks a question in plain language; the assistant finds relevant datasets,
+inspects their schema, writes and runs read-only SQL, and answers with both the
+result and the query it used.
+
+Superset ships **no model provider and talks to no model vendor by default**.
+The feature is disabled, and even when enabled it returns `404` until you point
+it at a provider you control. Nothing is sent anywhere until you configure it.
+
+## Enabling it
+
+Two things are required: the feature flag, and a provider.
+
+```python
+# superset_config.py
+FEATURE_FLAGS = {
+    "AI_ASSISTANT": True,
+}
+
+AI_LLM_PROVIDER_CLASS = "superset.ai.llm.anthropic.AnthropicProvider"
+AI_LLM_PROVIDER_CONFIG = {
+    "api_key": os.environ["ANTHROPIC_API_KEY"],
+    "models": {
+        "default": "claude-sonnet-4-5",
+        "fast": "claude-haiku-4-5",
+        "reasoning": "claude-opus-4-1",
+    },
+}
+```
+
+Install the matching extra:
+
+```bash
+pip install "apache-superset[ai-anthropic]"   # or [ai-openai]
+```
+
+Then run `superset init` so the assistant's permissions are created and 
assigned

Review Comment:
   An upgraded deployment that follows these enablement steps without 
separately running Alembic will create the permissions and then fail when the 
assistant accesses the new chat tables. Could this include `superset db 
upgrade` before `superset init`?



-- 
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]

Reply via email to