From c0b20f378c0d4533dc57990426fc301468ef9567 Mon Sep 17 00:00:00 2001 From: Michal Franczel Date: Tue, 25 Nov 2025 10:04:59 +0100 Subject: [PATCH 1/6] fix(installer): prioritize user-installed packages over server-libs Change sys.path.insert(0, ...) to sys.path.append(...) for server site-packages so user-installed packages in system site-packages take precedence over bundled server-libs packages. Previously, server-libs were inserted at the front of sys.path, causing bundled versions to override user-installed versions (e.g., streamlit). --- installer/module/virtual_environment.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/installer/module/virtual_environment.py b/installer/module/virtual_environment.py index d78799d..3eed41e 100644 --- a/installer/module/virtual_environment.py +++ b/installer/module/virtual_environment.py @@ -97,7 +97,7 @@ def import_package_bundle( # Write conditional import that checks environment variable pth_content = ( f"import os, sys; " - f"sys.path.insert(0, '{bundle_site_package_path}') " + f"sys.path.append('{bundle_site_package_path}') " f"if os.environ.get('{condition_env}', '').lower() == 'true' else None" ) pth_file.write(pth_content + "\n") From 28a70eccc8d29f6e26014e9e6cabe15ff1a4c0af Mon Sep 17 00:00:00 2001 From: Michal Franczel Date: Tue, 25 Nov 2025 10:04:59 +0100 Subject: [PATCH 2/6] fix(installer): prioritize user-installed packages over server-libs Change sys.path.insert(0, ...) to sys.path.append(...) for server site-packages so user-installed packages in system site-packages take precedence over bundled server-libs packages. Previously, server-libs were inserted at the front of sys.path, causing bundled versions to override user-installed versions (e.g., streamlit). --- installer/module/virtual_environment.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/installer/module/virtual_environment.py b/installer/module/virtual_environment.py index 3eed41e..54bbcb0 100644 --- a/installer/module/virtual_environment.py +++ b/installer/module/virtual_environment.py @@ -96,8 +96,9 @@ def import_package_bundle( if condition_env: # Write conditional import that checks environment variable pth_content = ( - f"import os, sys; " - f"sys.path.append('{bundle_site_package_path}') " + f"import sys, os, sysconfig; " + f"(sys.path.append('{bundle_site_package_path}'), " + f"sys.path.insert(sys.path.index('{bundle_site_package_path}'), sysconfig.get_path('purelib'))) " f"if os.environ.get('{condition_env}', '').lower() == 'true' else None" ) pth_file.write(pth_content + "\n") From cc4498a13b8268107fa4590df0cc1623a999a748 Mon Sep 17 00:00:00 2001 From: Michal Franczel Date: Tue, 25 Nov 2025 12:57:26 +0100 Subject: [PATCH 3/6] fix(installer): ensure system site-packages has priority over server-libs Add priority parameter to import_package_bundle that uses sys.path.insert(0) to place packages at the front of sys.path. System site-packages now uses this priority flag to ensure user-installed packages are found before bundled server-libs packages. --- installer/__main__.py | 2 +- installer/module/virtual_environment.py | 19 ++++++++++++++----- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/installer/__main__.py b/installer/__main__.py index b432fd9..fae2947 100644 --- a/installer/__main__.py +++ b/installer/__main__.py @@ -273,7 +273,7 @@ def bootstrap(): venv.import_package_bundle( server_site_packages_path, condition_env="DEEPNOTE_INCLUDE_SERVER_PACKAGES" ) - venv.import_package_bundle(system_site_packages_path) + venv.import_package_bundle(system_site_packages_path, priority=True) venv.import_package_bundle(kernel_site_package_path) # Phase 7.2: Symlink notebook to jupyter_server for compatibility with diff --git a/installer/module/virtual_environment.py b/installer/module/virtual_environment.py index 54bbcb0..3356efa 100644 --- a/installer/module/virtual_environment.py +++ b/installer/module/virtual_environment.py @@ -81,14 +81,17 @@ def start_server(self, command: str, cwd: Optional[str] = None) -> ServerProcess return server_proc def import_package_bundle( - self, bundle_site_package_path: str, condition_env: Optional[str] = None + self, bundle_site_package_path: str, + condition_env: Optional[str] = None, + priority: bool = False, ) -> None: """ Import a package bundle to the virtual environment. :param bundle_site_package_path: Absolute path to the package bundle. :param condition_env: Optional environment variable name. If provided, the bundle - will only be loaded when this env var is set to 'true'. + will only be loaded when this env var is set to 'true'. + :param priority: If True, insert at front of sys.path to override other bundles. """ pth_file_path = os.path.join(self._site_packages_path, "deepnote.pth") @@ -96,9 +99,8 @@ def import_package_bundle( if condition_env: # Write conditional import that checks environment variable pth_content = ( - f"import sys, os, sysconfig; " - f"(sys.path.append('{bundle_site_package_path}'), " - f"sys.path.insert(sys.path.index('{bundle_site_package_path}'), sysconfig.get_path('purelib'))) " + f"import os, sys; " + f"sys.path.insert(0, '{bundle_site_package_path}') " f"if os.environ.get('{condition_env}', '').lower() == 'true' else None" ) pth_file.write(pth_content + "\n") @@ -106,6 +108,13 @@ def import_package_bundle( "Bundle was conditionally imported to the virtual environment " f"(loads when {condition_env}=true)." ) + elif priority: + # Insert at front of sys.path for higher priority (overrides other bundles) + pth_content = f"import sys; sys.path.insert(0, '{bundle_site_package_path}')" + pth_file.write(pth_content + "\n") + logger.info( + "Bundle was imported with priority to the virtual environment." + ) else: pth_file.write(bundle_site_package_path + "\n") logger.info( From 2d369da5641a020d5a73f042bc6f80955a70ca3a Mon Sep 17 00:00:00 2001 From: Michal Franczel Date: Tue, 25 Nov 2025 14:57:23 +0100 Subject: [PATCH 4/6] test(installer): add tests for import_package_bundle priority param Add unit tests for VirtualEnvironment.import_package_bundle covering: - Plain path appending to .pth file - Conditional import with env var check - Priority flag using sys.path.insert(0, ...) - Ordering of bundles matching __main__.py usage --- installer/module/virtual_environment.py | 7 +- tests/unit/test_virtual_environment.py | 91 +++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 2 deletions(-) create mode 100644 tests/unit/test_virtual_environment.py diff --git a/installer/module/virtual_environment.py b/installer/module/virtual_environment.py index 3356efa..71316d9 100644 --- a/installer/module/virtual_environment.py +++ b/installer/module/virtual_environment.py @@ -81,7 +81,8 @@ def start_server(self, command: str, cwd: Optional[str] = None) -> ServerProcess return server_proc def import_package_bundle( - self, bundle_site_package_path: str, + self, + bundle_site_package_path: str, condition_env: Optional[str] = None, priority: bool = False, ) -> None: @@ -110,7 +111,9 @@ def import_package_bundle( ) elif priority: # Insert at front of sys.path for higher priority (overrides other bundles) - pth_content = f"import sys; sys.path.insert(0, '{bundle_site_package_path}')" + pth_content = ( + f"import sys; sys.path.insert(0, '{bundle_site_package_path}')" + ) pth_file.write(pth_content + "\n") logger.info( "Bundle was imported with priority to the virtual environment." diff --git a/tests/unit/test_virtual_environment.py b/tests/unit/test_virtual_environment.py new file mode 100644 index 0000000..68f6c01 --- /dev/null +++ b/tests/unit/test_virtual_environment.py @@ -0,0 +1,91 @@ +"""Tests for VirtualEnvironment class.""" + +import os +from unittest.mock import patch + +import pytest + +from installer.module.virtual_environment import VirtualEnvironment + + +class TestImportPackageBundle: + """Tests for import_package_bundle method.""" + + @pytest.fixture + def venv(self, tmp_path): + """Create a VirtualEnvironment with a temporary path.""" + venv_path = tmp_path / "venv" + with patch( + "installer.module.virtual_environment.get_current_python_version", + return_value="3.11", + ): + venv = VirtualEnvironment(str(venv_path)) + # Create the site-packages directory + os.makedirs(venv.site_packages_path, exist_ok=True) + return venv + + def test_import_package_bundle_plain_path(self, venv): + """Test that plain path is appended to .pth file.""" + bundle_path = "/some/bundle/site-packages" + + venv.import_package_bundle(bundle_path) + + pth_file = os.path.join(venv.site_packages_path, "deepnote.pth") + with open(pth_file, "r") as f: + content = f.read() + + assert content == f"{bundle_path}\n" + + def test_import_package_bundle_with_condition_env(self, venv): + """Test that conditional import uses sys.path.insert(0, ...).""" + bundle_path = "/server/libs/site-packages" + condition_env = "DEEPNOTE_INCLUDE_SERVER_PACKAGES" + + venv.import_package_bundle(bundle_path, condition_env=condition_env) + + pth_file = os.path.join(venv.site_packages_path, "deepnote.pth") + with open(pth_file, "r") as f: + content = f.read() + + assert "import os, sys" in content + assert f"sys.path.insert(0, '{bundle_path}')" in content + assert f"os.environ.get('{condition_env}', '').lower() == 'true'" in content + + def test_import_package_bundle_with_priority(self, venv): + """Test that priority=True uses sys.path.insert(0, ...).""" + bundle_path = "/usr/local/lib/python3.11/site-packages" + + venv.import_package_bundle(bundle_path, priority=True) + + pth_file = os.path.join(venv.site_packages_path, "deepnote.pth") + with open(pth_file, "r") as f: + content = f.read() + + assert "import sys" in content + assert f"sys.path.insert(0, '{bundle_path}')" in content + assert "os.environ.get" not in content + + def test_import_package_bundle_ordering(self, venv): + """Test .pth file content matches expected order from __main__.py.""" + server_libs = "/tmp/python3.11/server-libs/lib/python3.11/site-packages" + system_site = "/usr/local/lib/python3.11/site-packages" + kernel_libs = "/tmp/python3.11/kernel-libs/lib/python3.11/site-packages" + + venv.import_package_bundle( + server_libs, condition_env="DEEPNOTE_INCLUDE_SERVER_PACKAGES" + ) + venv.import_package_bundle(system_site, priority=True) + venv.import_package_bundle(kernel_libs) + + pth_file = os.path.join(venv.site_packages_path, "deepnote.pth") + with open(pth_file, "r") as f: + lines = f.readlines() + + assert len(lines) == 3 + # Line 1: server-libs conditional + assert "DEEPNOTE_INCLUDE_SERVER_PACKAGES" in lines[0] + assert f"sys.path.insert(0, '{server_libs}')" in lines[0] + # Line 2: system with priority + assert f"sys.path.insert(0, '{system_site}')" in lines[1] + # Line 3: kernel plain path + assert lines[2].strip() == kernel_libs From 4a9e8ebb24ea1ffb752abeb7f08f345390468b70 Mon Sep 17 00:00:00 2001 From: Michal Franczel Date: Tue, 25 Nov 2025 15:47:31 +0100 Subject: [PATCH 5/6] refactor(installer): improve import_package_bundle and tests - Add guard to raise ValueError when both condition_env and priority are specified (mutually exclusive parameters) - Update docstring to document the mutual exclusivity constraint - Refactor tests to use pathlib.Path instead of os.path/os.makedirs - Add test for mutually exclusive parameter validation --- installer/module/virtual_environment.py | 9 ++++++ tests/unit/test_virtual_environment.py | 37 +++++++++++++++---------- 2 files changed, 32 insertions(+), 14 deletions(-) diff --git a/installer/module/virtual_environment.py b/installer/module/virtual_environment.py index 71316d9..c6705ca 100644 --- a/installer/module/virtual_environment.py +++ b/installer/module/virtual_environment.py @@ -92,8 +92,17 @@ def import_package_bundle( :param bundle_site_package_path: Absolute path to the package bundle. :param condition_env: Optional environment variable name. If provided, the bundle will only be loaded when this env var is set to 'true'. + Cannot be combined with priority. :param priority: If True, insert at front of sys.path to override other bundles. + Cannot be combined with condition_env. + :raises ValueError: If both condition_env and priority are specified. """ + if condition_env and priority: + raise ValueError( + "condition_env and priority are mutually exclusive; " + "specify only one of them" + ) + pth_file_path = os.path.join(self._site_packages_path, "deepnote.pth") with open(pth_file_path, "a+", encoding="utf-8") as pth_file: diff --git a/tests/unit/test_virtual_environment.py b/tests/unit/test_virtual_environment.py index 68f6c01..841d0d9 100644 --- a/tests/unit/test_virtual_environment.py +++ b/tests/unit/test_virtual_environment.py @@ -1,6 +1,6 @@ """Tests for VirtualEnvironment class.""" -import os +from pathlib import Path from unittest.mock import patch import pytest @@ -21,7 +21,7 @@ def venv(self, tmp_path): ): venv = VirtualEnvironment(str(venv_path)) # Create the site-packages directory - os.makedirs(venv.site_packages_path, exist_ok=True) + Path(venv.site_packages_path).mkdir(parents=True, exist_ok=True) return venv def test_import_package_bundle_plain_path(self, venv): @@ -30,9 +30,8 @@ def test_import_package_bundle_plain_path(self, venv): venv.import_package_bundle(bundle_path) - pth_file = os.path.join(venv.site_packages_path, "deepnote.pth") - with open(pth_file, "r") as f: - content = f.read() + pth_file = Path(venv.site_packages_path) / "deepnote.pth" + content = pth_file.read_text() assert content == f"{bundle_path}\n" @@ -43,9 +42,8 @@ def test_import_package_bundle_with_condition_env(self, venv): venv.import_package_bundle(bundle_path, condition_env=condition_env) - pth_file = os.path.join(venv.site_packages_path, "deepnote.pth") - with open(pth_file, "r") as f: - content = f.read() + pth_file = Path(venv.site_packages_path) / "deepnote.pth" + content = pth_file.read_text() assert "import os, sys" in content assert f"sys.path.insert(0, '{bundle_path}')" in content @@ -57,9 +55,8 @@ def test_import_package_bundle_with_priority(self, venv): venv.import_package_bundle(bundle_path, priority=True) - pth_file = os.path.join(venv.site_packages_path, "deepnote.pth") - with open(pth_file, "r") as f: - content = f.read() + pth_file = Path(venv.site_packages_path) / "deepnote.pth" + content = pth_file.read_text() assert "import sys" in content assert f"sys.path.insert(0, '{bundle_path}')" in content @@ -77,9 +74,8 @@ def test_import_package_bundle_ordering(self, venv): venv.import_package_bundle(system_site, priority=True) venv.import_package_bundle(kernel_libs) - pth_file = os.path.join(venv.site_packages_path, "deepnote.pth") - with open(pth_file, "r") as f: - lines = f.readlines() + pth_file = Path(venv.site_packages_path) / "deepnote.pth" + lines = pth_file.read_text().splitlines() assert len(lines) == 3 # Line 1: server-libs conditional @@ -89,3 +85,16 @@ def test_import_package_bundle_ordering(self, venv): assert f"sys.path.insert(0, '{system_site}')" in lines[1] # Line 3: kernel plain path assert lines[2].strip() == kernel_libs + + def test_import_package_bundle_condition_env_and_priority_mutually_exclusive( + self, venv + ): + """Test that condition_env and priority cannot be used together.""" + bundle_path = "/some/bundle/site-packages" + + with pytest.raises(ValueError, match="mutually exclusive"): + venv.import_package_bundle( + bundle_path, + condition_env="SOME_ENV_VAR", + priority=True, + ) From 2021484ef1a5402f7f76bd1609f08898a82b5bb3 Mon Sep 17 00:00:00 2001 From: Michal Franczel Date: Tue, 25 Nov 2025 16:01:02 +0100 Subject: [PATCH 6/6] style(installer): make priority param keyword-only Add bare * before priority parameter to enforce keyword-only usage, satisfying Ruff FBT rules for boolean parameters. --- installer/module/virtual_environment.py | 1 + 1 file changed, 1 insertion(+) diff --git a/installer/module/virtual_environment.py b/installer/module/virtual_environment.py index c6705ca..841409a 100644 --- a/installer/module/virtual_environment.py +++ b/installer/module/virtual_environment.py @@ -84,6 +84,7 @@ def import_package_bundle( self, bundle_site_package_path: str, condition_env: Optional[str] = None, + *, priority: bool = False, ) -> None: """