fix: rabbit review 001 - persist Streamlit failures, guard Jupyter lifecycle
- st_demo: persist failure in call history, clear active_handle, and disable Run while call active to avoid overwriting handle - JupyterApp.shutdown: gather all shutdowns with return_exceptions=True and re-raise as BaseExceptionGroup so one failure doesn't leak kernels - messages: fallback known-but-malformed messages to UnknownJupyterMessage via ValidationError, make Unknown channel permissive (str) for nuance - transport: record reader failures as terminal state (_reader_failure) and fail fast from execute/messages_for instead of hanging - shell: don't reset execution_count to 0 when IPython returns None - README: use ipython_shell imports Skipped non-urgent/deferred items: line(), get_shell_2, test renames, import laziness, call-ID bounding, settle window config (worth doing later).
This commit is contained in:
@@ -3,8 +3,7 @@
|
|||||||
The application-facing API is:
|
The application-facing API is:
|
||||||
|
|
||||||
```python
|
```python
|
||||||
from ipython_demo import App
|
from ipython_shell import App, CallOptions
|
||||||
from ipython_demo import CallOptions
|
|
||||||
|
|
||||||
app = App()
|
app = App()
|
||||||
call = app.run_code(
|
call = app.run_code(
|
||||||
|
|||||||
@@ -1,3 +1,4 @@
|
|||||||
|
import asyncio
|
||||||
from collections.abc import AsyncIterator
|
from collections.abc import AsyncIterator
|
||||||
from datetime import UTC, datetime
|
from datetime import UTC, datetime
|
||||||
from time import monotonic
|
from time import monotonic
|
||||||
@@ -97,6 +98,10 @@ class JupyterApp:
|
|||||||
|
|
||||||
async def shutdown(self) -> None:
|
async def shutdown(self) -> None:
|
||||||
"""Stop all kernel processes owned by this app."""
|
"""Stop all kernel processes owned by this app."""
|
||||||
awaitables = [shell.shutdown() for shell in self.shells.values()]
|
results = await asyncio.gather(
|
||||||
for awaitable in awaitables:
|
*(shell.shutdown() for shell in self.shells.values()),
|
||||||
await awaitable
|
return_exceptions=True,
|
||||||
|
)
|
||||||
|
errors = [result for result in results if isinstance(result, BaseException)]
|
||||||
|
if errors:
|
||||||
|
raise BaseExceptionGroup("Kernel shutdown failed", errors)
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
from typing import Annotated, ClassVar, Final, Literal
|
from typing import Annotated, ClassVar, Final, Literal
|
||||||
|
|
||||||
from pydantic import BaseModel, ConfigDict, Field, TypeAdapter
|
from pydantic import BaseModel, ConfigDict, Field, TypeAdapter, ValidationError
|
||||||
|
|
||||||
|
|
||||||
type JupyterChannel = Literal["iopub", "shell", "stdin"]
|
type JupyterChannel = Literal["iopub", "shell", "stdin"]
|
||||||
@@ -176,6 +176,9 @@ class InputRequestMessage(_TypedMessage):
|
|||||||
class UnknownJupyterMessage(JupyterMessage):
|
class UnknownJupyterMessage(JupyterMessage):
|
||||||
"""An extension or future message preserved without lossy parsing."""
|
"""An extension or future message preserved without lossy parsing."""
|
||||||
|
|
||||||
|
# Allow any channel value so known-but-malformed fallbacks never fail
|
||||||
|
# on a strict Literal. The parser preserves the transport channel verbatim.
|
||||||
|
channel: str = "iopub" # type: ignore[assignment]
|
||||||
raw: dict[str, object] = Field(default_factory=dict)
|
raw: dict[str, object] = Field(default_factory=dict)
|
||||||
|
|
||||||
|
|
||||||
@@ -252,7 +255,14 @@ def parse_jupyter_message(
|
|||||||
|
|
||||||
if values["msg_type"] not in _KNOWN_MESSAGE_TYPES:
|
if values["msg_type"] not in _KNOWN_MESSAGE_TYPES:
|
||||||
return UnknownJupyterMessage.model_validate({**values, "raw": raw})
|
return UnknownJupyterMessage.model_validate({**values, "raw": raw})
|
||||||
return _KNOWN_MESSAGE_ADAPTER.validate_python(values)
|
try:
|
||||||
|
return _KNOWN_MESSAGE_ADAPTER.validate_python(values)
|
||||||
|
except ValidationError:
|
||||||
|
# Known msg_type but content or channel doesn't match the strict
|
||||||
|
# model (e.g., extra execution_state, wrong stdout/stderr name,
|
||||||
|
# or msg delivered on an unexpected channel). Degrade losslessly
|
||||||
|
# instead of aborting the whole call.
|
||||||
|
return UnknownJupyterMessage.model_validate({**values, "raw": raw})
|
||||||
|
|
||||||
|
|
||||||
# Existing callers use this name for the transport-level message. Keep it as
|
# Existing callers use this name for the transport-level message. Keep it as
|
||||||
|
|||||||
@@ -48,6 +48,7 @@ class JupyterTransport:
|
|||||||
asyncio.Queue[tuple[JupyterChannel, dict[str, Any] | BaseException]] | None
|
asyncio.Queue[tuple[JupyterChannel, dict[str, Any] | BaseException]] | None
|
||||||
) = None
|
) = None
|
||||||
self._reader_tasks: set[asyncio.Task[None]] = set()
|
self._reader_tasks: set[asyncio.Task[None]] = set()
|
||||||
|
self._reader_failure: BaseException | None = None
|
||||||
|
|
||||||
async def start(self) -> None:
|
async def start(self) -> None:
|
||||||
"""Start the kernel and its persistent channel readers."""
|
"""Start the kernel and its persistent channel readers."""
|
||||||
@@ -100,6 +101,7 @@ class JupyterTransport:
|
|||||||
raise
|
raise
|
||||||
except BaseException as error:
|
except BaseException as error:
|
||||||
# Surface reader failures instead of leaving messages_for() stuck.
|
# Surface reader failures instead of leaving messages_for() stuck.
|
||||||
|
self._reader_failure = error
|
||||||
await self._message_queue.put((channel, error))
|
await self._message_queue.put((channel, error))
|
||||||
|
|
||||||
async def _stop_readers(self) -> None:
|
async def _stop_readers(self) -> None:
|
||||||
@@ -113,6 +115,8 @@ class JupyterTransport:
|
|||||||
|
|
||||||
async def execute(self, code: str) -> str:
|
async def execute(self, code: str) -> str:
|
||||||
"""Submit one cell and return its Jupyter message ID."""
|
"""Submit one cell and return its Jupyter message ID."""
|
||||||
|
if self._reader_failure is not None:
|
||||||
|
raise RuntimeError("JupyterTransport channel reader failed") from self._reader_failure
|
||||||
if self.client is None:
|
if self.client is None:
|
||||||
raise RuntimeError("JupyterTransport has not been started")
|
raise RuntimeError("JupyterTransport has not been started")
|
||||||
|
|
||||||
@@ -126,6 +130,8 @@ class JupyterTransport:
|
|||||||
self, call_id: str
|
self, call_id: str
|
||||||
) -> AsyncIterator[ParsedJupyterMessage | InputRequest]:
|
) -> AsyncIterator[ParsedJupyterMessage | InputRequest]:
|
||||||
"""Yield decoded output, input, and completion messages for one call."""
|
"""Yield decoded output, input, and completion messages for one call."""
|
||||||
|
if self._reader_failure is not None:
|
||||||
|
raise RuntimeError("JupyterTransport channel reader failed") from self._reader_failure
|
||||||
if self.client is None:
|
if self.client is None:
|
||||||
raise RuntimeError("JupyterTransport has not been started")
|
raise RuntimeError("JupyterTransport has not been started")
|
||||||
if self._active_call != call_id:
|
if self._active_call != call_id:
|
||||||
|
|||||||
@@ -87,7 +87,8 @@ class Shell:
|
|||||||
else:
|
else:
|
||||||
self.status = "ready"
|
self.status = "ready"
|
||||||
self.last_used_at = datetime.now(UTC).isoformat()
|
self.last_used_at = datetime.now(UTC).isoformat()
|
||||||
self._last_execution_count = call_result.execution_count or 0
|
if call_result.execution_count is not None:
|
||||||
|
self._last_execution_count = call_result.execution_count
|
||||||
return call_result, events
|
return call_result, events
|
||||||
|
|
||||||
def describe(self) -> ShellInfo:
|
def describe(self) -> ShellInfo:
|
||||||
|
|||||||
+24
-1
@@ -227,6 +227,17 @@ def _render_active_call(controller: ShellController) -> None:
|
|||||||
events, finished, failed = controller.poll(handle)
|
events, finished, failed = controller.poll(handle)
|
||||||
records = call_records.setdefault(handle, [])
|
records = call_records.setdefault(handle, [])
|
||||||
records.extend(event_to_record(event) for event in events)
|
records.extend(event_to_record(event) for event in events)
|
||||||
|
# Restore persisted failure from a previous poll where the sentinel
|
||||||
|
# was already drained. Without this, the next poll returns
|
||||||
|
# failed=None, finished=False and shows "Running…" forever.
|
||||||
|
if failed is None:
|
||||||
|
failures = st.session_state.get("call_failures")
|
||||||
|
if isinstance(failures, dict):
|
||||||
|
cached = failures.get(handle)
|
||||||
|
if isinstance(cached, BaseException):
|
||||||
|
failed = cached # type: ignore[assignment]
|
||||||
|
elif isinstance(cached, Exception):
|
||||||
|
failed = cached
|
||||||
|
|
||||||
for record in visible_records(call_records):
|
for record in visible_records(call_records):
|
||||||
_render_record(record)
|
_render_record(record)
|
||||||
@@ -249,7 +260,17 @@ def _render_active_call(controller: ShellController) -> None:
|
|||||||
controller.reply_to_input(handle, call_id, value)
|
controller.reply_to_input(handle, call_id, value)
|
||||||
st.rerun(scope="fragment")
|
st.rerun(scope="fragment")
|
||||||
elif failed is not None:
|
elif failed is not None:
|
||||||
|
failures = st.session_state.get("call_failures")
|
||||||
|
if not isinstance(failures, dict):
|
||||||
|
failures = {}
|
||||||
|
st.session_state.call_failures = failures
|
||||||
|
failures[handle] = failed
|
||||||
|
# Persist in call history so the error remains visible after clearing.
|
||||||
|
error_text = f"{type(failed).__name__}: {failed}"
|
||||||
|
if not records or records[-1].get("text") != error_text:
|
||||||
|
records.append({"kind": "error", "text": error_text})
|
||||||
st.exception(failed)
|
st.exception(failed)
|
||||||
|
st.session_state.active_handle = None
|
||||||
elif not finished:
|
elif not finished:
|
||||||
st.caption("Running…")
|
st.caption("Running…")
|
||||||
else:
|
else:
|
||||||
@@ -276,7 +297,9 @@ def render() -> None:
|
|||||||
value="import matplotlib.pyplot as plt\nplt.plot([1, 2, 3], [4, 5, 6])",
|
value="import matplotlib.pyplot as plt\nplt.plot([1, 2, 3], [4, 5, 6])",
|
||||||
height=180,
|
height=180,
|
||||||
)
|
)
|
||||||
if st.button("Run", type="primary"):
|
active_handle = st.session_state.get("active_handle")
|
||||||
|
is_running = isinstance(active_handle, str)
|
||||||
|
if st.button("Run", type="primary", disabled=is_running):
|
||||||
handle = controller.start_call(code, shell_name)
|
handle = controller.start_call(code, shell_name)
|
||||||
st.session_state.active_handle = handle
|
st.session_state.active_handle = handle
|
||||||
st.session_state.call_records = getattr(
|
st.session_state.call_records = getattr(
|
||||||
|
|||||||
Reference in New Issue
Block a user