diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a41d9cf..c3dd0eb 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -151,64 +151,23 @@ $ ./bin/test-local ## Development Workflow for Deepnote maintainers -### Using in Deepnote Projects +### Local Toolkit development with Deepnote Cloud -When you push a commit, a new version of `deepnote/jupyter-for-local` is built with your commit hash (shortened!). Use it in projects by updating `common.yml`: +To develop deepnote-toolkit against a locally running Deepnote Cloud with hot-reload: -```yaml -jupyter: - image: "deepnote/jupyter-for-local:SHORTENED_COMMIT_SHA" -``` - -Alternatively, to develop against a local copy of Deepnote Toolkit, first run this command to build the image: - -```bash -docker build \ - --build-arg "FROM_PYTHON_TAG=3.11" \ - -t deepnote/deepnote-toolkit-local-hotreload \ - -f ./dockerfiles/jupyter-for-local-hotreload/Dockerfile . -``` - -Then start the container: - -```bash -# To include server logs in the output add this argument -# -e WITH_SERVER_LOGS=1 \ - -# Some toolkit features (e.g. feature flags support) require -# DEEPNOTE_PROJECT_ID to be set to work correctly. Add this -# argument with your project id -# -e DEEPNOTE_PROJECT_ID=981af2c1-fe8b-41b7-94bf-006b74cf0641 \ - -docker run \ - -v "$(pwd)":/deepnote-toolkit \ - -v /tmp/deepnote-mounts:/deepnote-mounts:shared \ - -p 8888:8888 \ - -p 2087:2087 \ - -p 8051:8051 \ - -w /deepnote-toolkit \ - --add-host=localstack.dev.deepnote.org:host-gateway \ - --rm \ - --name deepnote-toolkit-local-hotreload-container \ - deepnote/deepnote-toolkit-local-hotreload -``` - -This will start a container with Deepnote Toolkit mounted inside and expose all required ports. If you change code that runs in the kernel (e.g. you updated the DataFrame formatter), you only need to restart the kernel from Deepnote's UI. If you update code that starts Jupyter itself, you need to restart the container. And if you add or modify dependencies you need to rebuild the image. - -Now, you need to modify `common.yml` in the Deepnote app. First, replace `jupyter` service with noop image: +1. Build the local development image: + ```bash + docker build -t deepnote/jupyter-for-local:local -f ./dockerfiles/jupyter-for-local-hotreload/Dockerfile . + ``` -```yml -jupyter: - image: 'screwdrivercd/noop-container' -``` +2. Setup `DEEPNOTE_TOOLKIT_SOURCE_PATH` env variable pointing to folder with toolkit source. This can go either in `.zshrc` (or similar file for your shell) or set per shell session with `export DEEPNOTE_TOOLKIT_SOURCE_PATH=...`. If not set, Deepnote Cloud will try to resolve it to `../deepnote-toolkit` relative to Deepnote Cloud root folder. -And change `JUPYTER_HOST` variable of executor to point to host machine: +3. In the Deepnote Cloud repository, run: + ```bash + pnpm dev:app:local-toolkit + ``` -```yml -executor: - environment: - JUPYTER_HOST: host.docker.internal -``` +This mounts your toolkit source into the container and installs it in editable mode. Toolkit module code changes are reflected after kernel restart (use "Restart kernel" action in the Deepnote Cloud). ### Review Applications @@ -227,7 +186,9 @@ We use Docker to ensure reproducible environments due to Jupyter libraries' bina - `test.Dockerfile`: Provides consistent test environment for running unit and integration tests across Python versions using nox. Used both locally and in CI/CD pipeline. -- `jupyter-for-local.Dockerfile`: Creates development environment with Jupyter integration, used for local development from docker-compose used in main monorepo. +- `jupyter-for-local.Dockerfile`: Creates development environment with Jupyter integration, used for local development from docker-compose used in Deepnote Cloud. + +- `jupyter-for-local-hotreload.Dockerfile`: Creates development environment which expects toolkit source to be mounted at `/toolkit`. Used for development against locally running Deepnote Cloud by Deepnote employees. ### Production Releases diff --git a/deepnote_core/execution/registry.py b/deepnote_core/execution/registry.py index 528fdcf..9880f00 100644 --- a/deepnote_core/execution/registry.py +++ b/deepnote_core/execution/registry.py @@ -127,6 +127,10 @@ def _execute_jupyter_server( if action.no_browser: argv.append("--no-browser") + # Set root directory if specified (affects Jupyter's file browser and API paths) + if action.root_dir: + argv.append(f"--ServerApp.root_dir={action.root_dir}") + # Add any extra arguments if action.extra_args: argv.extend(action.extra_args) diff --git a/deepnote_core/runtime/plan.py b/deepnote_core/runtime/plan.py index a54bd92..6c017cb 100644 --- a/deepnote_core/runtime/plan.py +++ b/deepnote_core/runtime/plan.py @@ -45,12 +45,17 @@ def build_server_plan(cfg: DeepnoteConfig) -> List[RuntimeAction]: else False ) + # Determine Jupyter root directory from config + # This affects what paths the Jupyter API returns for notebooks + root_dir = str(cfg.paths.notebook_root) if cfg.paths.notebook_root else None + actions.append( JupyterServerSpec( port=cfg.server.jupyter_port, allow_root=allow_root, enable_terminals=cfg.server.enable_terminals, no_browser=True, + root_dir=root_dir, extra_args=[], ) ) diff --git a/deepnote_core/runtime/types.py b/deepnote_core/runtime/types.py index ee0e522..aa5d56e 100644 --- a/deepnote_core/runtime/types.py +++ b/deepnote_core/runtime/types.py @@ -17,6 +17,9 @@ class Config: no_browser: bool = Field(default=True, description="Disable browser auto-open") allow_root: bool = Field(default=False, description="Allow root execution") enable_terminals: bool = Field(default=True, description="Enable terminal support") + root_dir: Optional[str] = Field( + default=None, description="Root directory for Jupyter file browser and API" + ) extra_args: List[str] = Field( default_factory=list, description="Additional arguments" ) diff --git a/deepnote_toolkit/set_notebook_path.py b/deepnote_toolkit/set_notebook_path.py index af74f01..d3e7c4e 100644 --- a/deepnote_toolkit/set_notebook_path.py +++ b/deepnote_toolkit/set_notebook_path.py @@ -9,6 +9,7 @@ from . import env from .config import get_config +from .logging import get_logger def set_notebook_path() -> None: @@ -107,5 +108,7 @@ def set_notebook_path() -> None: if notebook_directory not in sys.path: sys.path.append(notebook_directory) os.chdir(notebook_directory) - except Exception: # pylint: disable=broad-except + get_logger().info("Kernel working directory set to: %s", notebook_directory) + except Exception as e: # pylint: disable=broad-except + get_logger().error("Failed to set notebook path: %s", e) traceback.print_exc() diff --git a/dockerfiles/jupyter-for-local-hotreload/Dockerfile b/dockerfiles/jupyter-for-local-hotreload/Dockerfile index 4d25eb9..d175d21 100644 --- a/dockerfiles/jupyter-for-local-hotreload/Dockerfile +++ b/dockerfiles/jupyter-for-local-hotreload/Dockerfile @@ -1,32 +1,57 @@ -ARG FROM_PYTHON_TAG +# Dockerfile for local development with hot-reload support +# This container expects the toolkit source to be mounted at /toolkit +# and installs it in editable mode for live code changes +# +# Build with: +# docker build -t deepnote/jupyter-for-local:local -f dockerfiles/jupyter-for-local-hotreload/Dockerfile . + +ARG FROM_PYTHON_TAG=3.12 FROM deepnote/python:${FROM_PYTHON_TAG} +ARG FROM_PYTHON_TAG + +ENV DEBIAN_FRONTEND=noninteractive + +# Install system dependencies RUN apt-get update && \ - apt-get install -y openjdk-17-jdk && \ + apt-get install --no-install-recommends -y \ + rsync \ + git \ + # Required for pymssql + freetds-dev \ + # Required for database connectivity through ODBC + unixodbc-dev \ + # Required for secure connections (SSL/TLS) + libssl-dev && \ apt-get clean && \ rm -rf /var/lib/apt/lists/* -RUN pip install poetry==2.2.0 +# Install Poetry +RUN pip install --no-cache-dir poetry==2.2.0 -WORKDIR /deepnote-toolkit +# Configure Poetry to create virtualenv outside the mounted source directory +# - virtualenvs.in-project false: Never use .venv from mounted host directory +# - virtualenvs.path: Store venvs in container-local directory +RUN poetry config virtualenvs.in-project false && \ + poetry config virtualenvs.path /opt/venvs -ENV POETRY_NO_INTERACTION=1 \ - POETRY_VIRTUALENVS_CREATE=1 \ - POETRY_VIRTUALENVS_IN_PROJECT=0 \ - POETRY_CACHE_DIR=/tmp/poetry_cache +# Create toolkit directory (will be mounted over, but needed for initial setup) +RUN mkdir -p /toolkit /opt/venvs -COPY pyproject.toml poetry.lock poetry.toml ./ +WORKDIR /toolkit -RUN poetry install --no-interaction --no-ansi --with server --with dev +# Environment variables for development mode +# POETRY_VIRTUALENVS_* ensures we never use host's .venv even if poetry.toml exists +ENV DEEPNOTE_RUNNING_IN_DEV_MODE=true \ + PYTHONDONTWRITEBYTECODE=1 \ + PYTHONUNBUFFERED=1 \ + POETRY_VIRTUALENVS_IN_PROJECT=false \ + POETRY_VIRTUALENVS_PATH=/opt/venvs -ENV PYTHONPATH=/deepnote-toolkit:/deepnote-toolkit/installer:$PYTHONPATH \ - TOOLKIT_BUNDLE_PATH=/deepnote-toolkit \ - TOOLKIT_VERSION="local-build" \ - USERNAME=user \ - PASSWORD=password \ - DEEPNOTE_RUNNING_IN_DEV_MODE=true \ - DEEPNOTE_WEBAPP_URL="http://host.docker.internal:3002" +# Copy the entrypoint script +COPY dockerfiles/jupyter-for-local-hotreload/entrypoint.sh /entrypoint.sh +RUN chmod +x /entrypoint.sh -COPY dockerfiles/jupyter-for-local-hotreload/run-installer.sh /usr/local/bin/run-installer.sh +EXPOSE 8888 -ENTRYPOINT ["/usr/local/bin/run-installer.sh"] +ENTRYPOINT ["/entrypoint.sh"] diff --git a/dockerfiles/jupyter-for-local-hotreload/entrypoint.sh b/dockerfiles/jupyter-for-local-hotreload/entrypoint.sh new file mode 100644 index 0000000..b82812c --- /dev/null +++ b/dockerfiles/jupyter-for-local-hotreload/entrypoint.sh @@ -0,0 +1,71 @@ +#!/bin/bash +set -e + +# Entrypoint script for local development container +# Sets up filesystem mounts, installs toolkit in editable mode, and starts servers + +echo "[local-toolkit] Starting local development environment..." + +# Check if toolkit source is mounted +if [ ! -f "/toolkit/pyproject.toml" ]; then + echo "[local-toolkit] ERROR: Toolkit source not found at /toolkit" + echo "[local-toolkit] Make sure to mount the deepnote-toolkit directory to /toolkit" + exit 1 +fi + +# Wait for s3fs mount to be available (created by localstack container) +echo "[local-toolkit] Waiting for /deepnote-mounts/s3fs to be created..." +while [ ! -d /deepnote-mounts/s3fs ]; do + sleep 2 +done +echo "[local-toolkit] /deepnote-mounts/s3fs is available" + +mkdir -p /datasets +ln -sf /deepnote-mounts/s3fs /datasets/_deepnote_work +echo "[local-toolkit] Created /datasets/_deepnote_work symlink" + +# Create /work symlink pointing to project-specific path +# In dev mode with PROJECT_ID, use project-specific path under s3fs +if [ -n "$PROJECT_ID" ]; then + PROJECT_WORK_PATH="/datasets/_deepnote_work/projects/${PROJECT_ID}" + mkdir -p "$PROJECT_WORK_PATH" + ln -sf "$PROJECT_WORK_PATH" /work + echo "[local-toolkit] Created /work -> $PROJECT_WORK_PATH symlink" +else + ln -sf /datasets/_deepnote_work /work + echo "[local-toolkit] Created /work -> /datasets/_deepnote_work symlink" +fi + +cd /toolkit + +# Install dependencies and toolkit in editable mode +echo "[local-toolkit] Installing toolkit in editable mode..." +poetry install --extras server --no-interaction + +echo "[local-toolkit] Starting servers from /work directory..." + +# Create log directory and start tailing the log file in background +# This makes toolkit logs visible in docker container output +LOG_FILE="/root/.local/state/deepnote-toolkit/logs/helpers.log" +mkdir -p "$(dirname "$LOG_FILE")" +touch "$LOG_FILE" +tail -f "$LOG_FILE" & +TAIL_PID=$! + +# Clean up tail process on exit +cleanup() { + if kill -0 "$TAIL_PID" 2>/dev/null; then + kill "$TAIL_PID" 2>/dev/null || true + fi +} +trap cleanup EXIT SIGINT SIGTERM + +# Configure Jupyter to use /work as its root directory +# This is picked up by the config loader and passed to Jupyter's --ServerApp.root_dir +export DEEPNOTE_PATHS__NOTEBOOK_ROOT=/work + +cd /work + +# Run server in foreground (not exec) so trap can clean up tail process +poetry --directory /toolkit run deepnote-toolkit server "$@" +exit $? diff --git a/dockerfiles/jupyter-for-local-hotreload/run-installer.sh b/dockerfiles/jupyter-for-local-hotreload/run-installer.sh deleted file mode 100755 index 450f28f..0000000 --- a/dockerfiles/jupyter-for-local-hotreload/run-installer.sh +++ /dev/null @@ -1,42 +0,0 @@ -#!/bin/bash - -set -euo pipefail - -while [ ! -d /deepnote-mounts/s3fs ]; do - echo "Waiting for /deepnote-mounts/s3fs to be created..." - sleep 2 -done - -mkdir -p /datasets -ln -sf /deepnote-mounts/s3fs /datasets/_deepnote_work -mkdir -p /var/log/deepnote -touch /var/log/deepnote/helpers.log - -handle_sigint() { - echo "Received SIGINT, shutting down..." - # Send TERM to all processes in our process group except ourselves - echo "Sending TERM to all remaining child processes..." - pkill -P $$ 2>/dev/null - - # Final cleanup - forcefully kill any remaining child processes - sleep 2 - echo "Sending KILL to any remaining processes..." - pkill -9 -P $$ 2>/dev/null - exit 0 -} - -trap handle_sigint SIGINT - -echo "Starting installer..." -if [ -n "${WITH_SERVER_LOGS:-}" ]; then - poetry run python -m installer --venv-path "$(poetry env info --path)" & -else - poetry run python -m installer --venv-path "$(poetry env info --path)" > /dev/null 2>&1 & -fi -installer_pid=$! - -echo "Starting log tail..." -tail -f /var/log/deepnote/helpers.log & -tail_pid=$! - -wait $installer_pid $tail_pid diff --git a/tests/unit/test_action_registry.py b/tests/unit/test_action_registry.py index d0df655..23f9d4c 100644 --- a/tests/unit/test_action_registry.py +++ b/tests/unit/test_action_registry.py @@ -210,6 +210,40 @@ def test_jupyter_server_without_token_warning(self): mock_context.logger.warning.assert_called_once() assert "insecure" in mock_context.logger.warning.call_args[0][0] + def test_jupyter_server_with_root_dir(self): + """Test JupyterServerSpec with custom root directory.""" + action = JupyterServerSpec( + host="0.0.0.0", + port=8888, + root_dir="/work", + ) + + mock_context = mock.Mock() + mock_context.python_executable.return_value = "python" + mock_context.logger = logging.getLogger("test") + mock_proc = mock.Mock() + mock_context.spawn.return_value = mock_proc + + result = execute_action(action, mock_context) + + assert result.success is True + assert result.is_long_running is True + + # Verify the command includes --ServerApp.root_dir + expected_argv = [ + "python", + "-m", + "jupyter", + "server", + "--ip", + "0.0.0.0", + "--port", + "8888", + "--no-browser", + "--ServerApp.root_dir=/work", + ] + mock_context.spawn.assert_called_once_with(expected_argv, env_override={}) + def test_python_lsp_action(self): """Test PythonLSPSpec execution.""" action = PythonLSPSpec(host="localhost", port=2087, verbose=True) diff --git a/tests/unit/test_runtime_plan.py b/tests/unit/test_runtime_plan.py index e774030..7b4bf9c 100644 --- a/tests/unit/test_runtime_plan.py +++ b/tests/unit/test_runtime_plan.py @@ -57,6 +57,26 @@ def test_jupyter_server_with_terminals(self): assert actions[1].no_browser is True assert actions[1].host == "0.0.0.0" + def test_jupyter_server_with_root_dir(self): + """Test Jupyter server with custom root directory.""" + from pathlib import Path + + cfg = mock.MagicMock() + cfg.server.start_jupyter = True + cfg.server.jupyter_port = 8888 + cfg.server.enable_terminals = False + cfg.server.start_ls = False + cfg.server.start_streamlit_servers = False + cfg.server.start_extra_servers = False + cfg.installation.install_method = "pip" + cfg.paths.notebook_root = Path("/custom/root") + + actions = build_server_plan(cfg) + + assert len(actions) == 1 + assert isinstance(actions[0], JupyterServerSpec) + assert actions[0].root_dir == "/custom/root" + def test_python_lsp_server(self): """Test Python LSP server configuration.""" cfg = mock.MagicMock() diff --git a/tests/unit/test_set_notebook_path.py b/tests/unit/test_set_notebook_path.py index 7288e0c..79e9c1e 100644 --- a/tests/unit/test_set_notebook_path.py +++ b/tests/unit/test_set_notebook_path.py @@ -1,8 +1,10 @@ import importlib import json +import logging import os import types from pathlib import Path +from unittest import mock from deepnote_toolkit import env as dnenv from deepnote_toolkit.set_notebook_path import set_notebook_path @@ -83,3 +85,28 @@ def fake_chdir(p): assert seen["url"] == "http://0.0.0.0:9999/api/sessions" assert seen["headers"] is not None assert seen["headers"]["Authorization"] == "token tok" + + +def test_set_notebook_path_logs_error_on_failure(monkeypatch, capsys): + """Test that exceptions are caught and logged properly.""" + mod = importlib.import_module("deepnote_toolkit.set_notebook_path") + + # Make get_connection_file raise an exception + monkeypatch.setattr( + "ipykernel.connect.get_connection_file", + mock.Mock(side_effect=RuntimeError("No kernel connection")), + ) + + # Mock the logger to capture log calls + mock_logger = mock.Mock(spec=logging.Logger) + monkeypatch.setattr(mod, "get_logger", lambda: mock_logger) + + # Call - should not raise + set_notebook_path() + + # Verify error was logged + mock_logger.error.assert_called_once() + call_args = mock_logger.error.call_args[0] + assert "Failed to set notebook path" in call_args[0] + assert isinstance(call_args[1], RuntimeError) + assert "No kernel connection" in str(call_args[1])