Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions deepnote_core/execution/registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
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)
Expand Down
8 changes: 8 additions & 0 deletions installer/module/streamlit.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
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.
Expand Down
21 changes: 21 additions & 0 deletions tests/unit/test_action_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
23 changes: 8 additions & 15 deletions tests/unit/test_installer_executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)."""
Expand Down
55 changes: 54 additions & 1 deletion tests/unit/test_streamlit.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -44,3 +44,56 @@ def test_fetch_streamlit_apps(self):
}
],
)


class TestStartStreamlitServers(unittest.TestCase):
def _make_apps(self, *entrypoints: str) -> list:
Comment thread
robertlacok marked this conversation as resolved.
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
Loading