From 086ecc9ff1ed3081f6b115d9d46a7ba8451512a1 Mon Sep 17 00:00:00 2001 From: Robert Date: Wed, 17 Jun 2026 14:37:01 +0200 Subject: [PATCH 1/4] fix: skip Streamlit apps whose entrypoint directory no longer exists Co-Authored-By: Claude Sonnet 4.6 --- deepnote_core/execution/registry.py | 8 +++++ installer/module/streamlit.py | 6 ++++ tests/unit/test_action_registry.py | 21 +++++++++++++ tests/unit/test_streamlit.py | 47 +++++++++++++++++++++++++++-- 4 files changed, 80 insertions(+), 2 deletions(-) diff --git a/deepnote_core/execution/registry.py b/deepnote_core/execution/registry.py index 9880f00b..18222ad3 100644 --- a/deepnote_core/execution/registry.py +++ b/deepnote_core/execution/registry.py @@ -244,6 +244,14 @@ def _execute_streamlit( if parent != Path("."): cwd = str(parent) + # The entrypoint's parent directory may no longer exist if the folder was deleted + # from the project after the app was registered. Skip rather than crash the installer. + if cwd is not None and not Path(cwd).exists(): + context.logger.warning( + f"Skipping Streamlit app {action.script!r}: directory {cwd!r} does not exist" + ) + return ExecutionResult(process=None, is_long_running=False, success=False) + # Start the Streamlit app context.logger.info(f"Starting Streamlit app: {action.script}") proc = context.spawn(argv, env_override=None, cwd=cwd) diff --git a/installer/module/streamlit.py b/installer/module/streamlit.py index 70906b6c..67da3280 100644 --- a/installer/module/streamlit.py +++ b/installer/module/streamlit.py @@ -92,6 +92,12 @@ def start_streamlit_servers( entrypoint_path = f"/work/{entrypoint}" directory_path = os.path.dirname(entrypoint_path) + if not os.path.exists(directory_path): + logger.warning( + f"Skipping Streamlit app {entrypoint_path!r}: directory {directory_path!r} does not exist" + ) + continue + # We need to disable CORS because we access the Streamlit server from a different domain # via an iframe, through a proxy. We also disable XSRF protection because the Streamlit # docs says it should be disabled when CORS is disabled. diff --git a/tests/unit/test_action_registry.py b/tests/unit/test_action_registry.py index 23f9d4c4..1dec9d95 100644 --- a/tests/unit/test_action_registry.py +++ b/tests/unit/test_action_registry.py @@ -314,6 +314,27 @@ def test_streamlit_action(self): "streamlit", "pip install streamlit" ) + def test_streamlit_action_missing_directory(self, tmp_path): + """Test StreamlitSpec returns a failed result when the entrypoint directory is missing.""" + nonexistent_dir = tmp_path / "deleted_folder" + action = StreamlitSpec( + script=str(nonexistent_dir / "app.py"), + port=8501, + ) + + mock_context = mock.Mock() + mock_context.python_executable.return_value = "python" + mock_context.logger = mock.Mock(spec=logging.Logger) + + result = execute_action(action, mock_context) + + assert result.success is False + assert result.is_long_running is False + assert result.process is None + mock_context.spawn.assert_not_called() + mock_context.logger.warning.assert_called_once() + assert "does not exist" in mock_context.logger.warning.call_args[0][0] + def test_streamlit_action_no_port(self): """Test StreamlitSpec with auto-assigned port.""" action = StreamlitSpec(script="app.py") diff --git a/tests/unit/test_streamlit.py b/tests/unit/test_streamlit.py index 9c4fd0eb..755bb595 100644 --- a/tests/unit/test_streamlit.py +++ b/tests/unit/test_streamlit.py @@ -1,9 +1,9 @@ import json import logging import unittest -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock, call, patch -from installer.module.streamlit import fetch_streamlit_apps +from installer.module.streamlit import fetch_streamlit_apps, start_streamlit_servers class TestFetchStreamlitApps(unittest.TestCase): @@ -44,3 +44,46 @@ def test_fetch_streamlit_apps(self): } ], ) + + +class TestStartStreamlitServers(unittest.TestCase): + def _make_apps(self, *entrypoints): + return [ + {"id": f"id-{i}", "entrypoint": ep, "port": str(8501 + i), "projectId": "p"} + for i, ep in enumerate(entrypoints) + ] + + def test_skips_app_when_directory_missing(self): + """Apps whose parent directory does not exist are skipped without crashing.""" + apps = self._make_apps("deleted_folder/app.py", "existing_folder/app.py") + mock_venv = MagicMock() + mock_logger = MagicMock(spec=logging.Logger) + + def exists_side_effect(path): + return "deleted_folder" not in path + + with patch("installer.module.streamlit.fetch_streamlit_apps", return_value=apps): + with patch("installer.module.streamlit.os.path.exists", side_effect=exists_side_effect): + start_streamlit_servers(mock_venv, mock_logger) + + mock_logger.warning.assert_called_once() + assert "deleted_folder" in mock_logger.warning.call_args[0][0] + assert mock_venv.start_server.call_count == 1 + started_cmd = mock_venv.start_server.call_args[0][0] + assert "existing_folder/app.py" in started_cmd + + def test_continues_after_skipped_app(self): + """A missing-directory app does not prevent subsequent apps from starting.""" + apps = self._make_apps("gone/app.py", "also_gone/app.py", "present/app.py") + mock_venv = MagicMock() + mock_logger = MagicMock(spec=logging.Logger) + + def exists_side_effect(path): + return "present" in path + + with patch("installer.module.streamlit.fetch_streamlit_apps", return_value=apps): + with patch("installer.module.streamlit.os.path.exists", side_effect=exists_side_effect): + start_streamlit_servers(mock_venv, mock_logger) + + assert mock_logger.warning.call_count == 2 + assert mock_venv.start_server.call_count == 1 From 7e46b57936072c5b8efcf2868a02b2c8d029b6dc Mon Sep 17 00:00:00 2001 From: Robert Date: Wed, 17 Jun 2026 14:47:16 +0200 Subject: [PATCH 2/4] fix: address coderabbit review comments - Use lazy %r logger formatting (G004) in registry and streamlit module - Fix test_streamlit_with_directory_path to create a real tmp dir so the new directory-existence guard does not skip the action - Flatten nested with-patch blocks (SIM117) in test_streamlit.py - Add type hints to test helpers in test_streamlit.py Co-Authored-By: Claude Sonnet 4.6 --- deepnote_core/execution/registry.py | 4 +++- installer/module/streamlit.py | 4 +++- tests/unit/test_installer_executor.py | 23 ++++++++--------------- tests/unit/test_streamlit.py | 24 ++++++++++++++---------- 4 files changed, 28 insertions(+), 27 deletions(-) diff --git a/deepnote_core/execution/registry.py b/deepnote_core/execution/registry.py index 18222ad3..dbc3c51e 100644 --- a/deepnote_core/execution/registry.py +++ b/deepnote_core/execution/registry.py @@ -248,7 +248,9 @@ def _execute_streamlit( # from the project after the app was registered. Skip rather than crash the installer. if cwd is not None and not Path(cwd).exists(): context.logger.warning( - f"Skipping Streamlit app {action.script!r}: directory {cwd!r} does not exist" + "Skipping Streamlit app %r: directory %r does not exist", + action.script, + cwd, ) return ExecutionResult(process=None, is_long_running=False, success=False) diff --git a/installer/module/streamlit.py b/installer/module/streamlit.py index 67da3280..c20c1aee 100644 --- a/installer/module/streamlit.py +++ b/installer/module/streamlit.py @@ -94,7 +94,9 @@ def start_streamlit_servers( if not os.path.exists(directory_path): logger.warning( - f"Skipping Streamlit app {entrypoint_path!r}: directory {directory_path!r} does not exist" + "Skipping Streamlit app %r: directory %r does not exist", + entrypoint_path, + directory_path, ) continue diff --git a/tests/unit/test_installer_executor.py b/tests/unit/test_installer_executor.py index 35f7b704..a8b41f53 100644 --- a/tests/unit/test_installer_executor.py +++ b/tests/unit/test_installer_executor.py @@ -470,32 +470,25 @@ def test_command_with_spaces(self): expected_cmd = shlex.join(["python", "my script.py", "--title", "Test Server"]) mock_venv.start_server.assert_called_once_with(expected_cmd, cwd=None) - def test_streamlit_with_directory_path(self): + def test_streamlit_with_directory_path(self, tmp_path): """Test Streamlit server with script in a directory.""" + apps_dir = tmp_path / "apps" + apps_dir.mkdir() + script_path = str(apps_dir / "dashboard.py") + mock_venv = mock.MagicMock() mock_process = mock.MagicMock() mock_venv.start_server.return_value = mock_process - action = StreamlitSpec( - script="/work/apps/dashboard.py", - port=8501, - ) + action = StreamlitSpec(script=script_path, port=8501) processes = run_actions_in_installer_env(mock_venv, [action]) assert len(processes) == 1 expected_cmd = shlex.join( - [ - "python", - "-m", - "streamlit", - "run", - "/work/apps/dashboard.py", - "--server.port", - "8501", - ] + ["python", "-m", "streamlit", "run", script_path, "--server.port", "8501"] ) - mock_venv.start_server.assert_called_once_with(expected_cmd, cwd="/work/apps") + mock_venv.start_server.assert_called_once_with(expected_cmd, cwd=str(apps_dir)) def test_streamlit_none_port(self): """Test Streamlit with None port (should not add --server.port).""" diff --git a/tests/unit/test_streamlit.py b/tests/unit/test_streamlit.py index 755bb595..44760aee 100644 --- a/tests/unit/test_streamlit.py +++ b/tests/unit/test_streamlit.py @@ -47,7 +47,7 @@ def test_fetch_streamlit_apps(self): class TestStartStreamlitServers(unittest.TestCase): - def _make_apps(self, *entrypoints): + def _make_apps(self, *entrypoints: str) -> list: return [ {"id": f"id-{i}", "entrypoint": ep, "port": str(8501 + i), "projectId": "p"} for i, ep in enumerate(entrypoints) @@ -59,15 +59,17 @@ def test_skips_app_when_directory_missing(self): mock_venv = MagicMock() mock_logger = MagicMock(spec=logging.Logger) - def exists_side_effect(path): + def exists_side_effect(path: str) -> bool: return "deleted_folder" not in path - with patch("installer.module.streamlit.fetch_streamlit_apps", return_value=apps): - with patch("installer.module.streamlit.os.path.exists", side_effect=exists_side_effect): - start_streamlit_servers(mock_venv, mock_logger) + with ( + patch("installer.module.streamlit.fetch_streamlit_apps", return_value=apps), + patch("installer.module.streamlit.os.path.exists", side_effect=exists_side_effect), + ): + start_streamlit_servers(mock_venv, mock_logger) mock_logger.warning.assert_called_once() - assert "deleted_folder" in mock_logger.warning.call_args[0][0] + assert any("deleted_folder" in str(a) for a in mock_logger.warning.call_args[0]) assert mock_venv.start_server.call_count == 1 started_cmd = mock_venv.start_server.call_args[0][0] assert "existing_folder/app.py" in started_cmd @@ -78,12 +80,14 @@ def test_continues_after_skipped_app(self): mock_venv = MagicMock() mock_logger = MagicMock(spec=logging.Logger) - def exists_side_effect(path): + def exists_side_effect(path: str) -> bool: return "present" in path - with patch("installer.module.streamlit.fetch_streamlit_apps", return_value=apps): - with patch("installer.module.streamlit.os.path.exists", side_effect=exists_side_effect): - start_streamlit_servers(mock_venv, mock_logger) + with ( + patch("installer.module.streamlit.fetch_streamlit_apps", return_value=apps), + patch("installer.module.streamlit.os.path.exists", side_effect=exists_side_effect), + ): + start_streamlit_servers(mock_venv, mock_logger) assert mock_logger.warning.call_count == 2 assert mock_venv.start_server.call_count == 1 From cc17e443054988553661c2e8c8603c1aa7baf68e Mon Sep 17 00:00:00 2001 From: Robert Date: Wed, 17 Jun 2026 14:54:28 +0200 Subject: [PATCH 3/4] style: apply black formatting to test_streamlit.py Co-Authored-By: Claude Sonnet 4.6 --- tests/unit/test_streamlit.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_streamlit.py b/tests/unit/test_streamlit.py index 44760aee..f65fea04 100644 --- a/tests/unit/test_streamlit.py +++ b/tests/unit/test_streamlit.py @@ -64,7 +64,10 @@ def exists_side_effect(path: str) -> bool: with ( patch("installer.module.streamlit.fetch_streamlit_apps", return_value=apps), - patch("installer.module.streamlit.os.path.exists", side_effect=exists_side_effect), + patch( + "installer.module.streamlit.os.path.exists", + side_effect=exists_side_effect, + ), ): start_streamlit_servers(mock_venv, mock_logger) @@ -85,7 +88,10 @@ def exists_side_effect(path: str) -> bool: with ( patch("installer.module.streamlit.fetch_streamlit_apps", return_value=apps), - patch("installer.module.streamlit.os.path.exists", side_effect=exists_side_effect), + patch( + "installer.module.streamlit.os.path.exists", + side_effect=exists_side_effect, + ), ): start_streamlit_servers(mock_venv, mock_logger) From c140e7b3fca222de7225044a2d4fa9eb69007445 Mon Sep 17 00:00:00 2001 From: Robert Date: Wed, 17 Jun 2026 16:25:56 +0200 Subject: [PATCH 4/4] fix: remove unused call import in test_streamlit.py Co-Authored-By: Claude Sonnet 4.6 --- tests/unit/test_streamlit.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_streamlit.py b/tests/unit/test_streamlit.py index f65fea04..e3dfb7ec 100644 --- a/tests/unit/test_streamlit.py +++ b/tests/unit/test_streamlit.py @@ -1,7 +1,7 @@ import json import logging import unittest -from unittest.mock import MagicMock, call, patch +from unittest.mock import MagicMock, patch from installer.module.streamlit import fetch_streamlit_apps, start_streamlit_servers