diff --git a/deepnote_core/execution/registry.py b/deepnote_core/execution/registry.py index 9880f00..dbc3c51 100644 --- a/deepnote_core/execution/registry.py +++ b/deepnote_core/execution/registry.py @@ -244,6 +244,16 @@ 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( + "Skipping Streamlit app %r: directory %r does not exist", + action.script, + cwd, + ) + 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 70906b6..c20c1ae 100644 --- a/installer/module/streamlit.py +++ b/installer/module/streamlit.py @@ -92,6 +92,14 @@ 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( + "Skipping Streamlit app %r: directory %r does not exist", + entrypoint_path, + directory_path, + ) + 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 23f9d4c..1dec9d9 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_installer_executor.py b/tests/unit/test_installer_executor.py index 35f7b70..a8b41f5 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 9c4fd0e..e3dfb7e 100644 --- a/tests/unit/test_streamlit.py +++ b/tests/unit/test_streamlit.py @@ -3,7 +3,7 @@ import unittest from unittest.mock import MagicMock, 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,56 @@ def test_fetch_streamlit_apps(self): } ], ) + + +class TestStartStreamlitServers(unittest.TestCase): + 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) + ] + + 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: str) -> bool: + return "deleted_folder" not in path + + 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 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 + + 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: str) -> bool: + return "present" in path + + 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