Skip to content

Commit a251c6b

Browse files
authored
fix(installer): Prioritize system site packages over server-libs (#38)
* 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). * 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). * 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. * 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 * 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 * style(installer): make priority param keyword-only Add bare * before priority parameter to enforce keyword-only usage, satisfying Ruff FBT rules for boolean parameters.
1 parent 4d48a52 commit a251c6b

File tree

3 files changed

+126
-3
lines changed

3 files changed

+126
-3
lines changed

installer/__main__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ def bootstrap():
273273
venv.import_package_bundle(
274274
server_site_packages_path, condition_env="DEEPNOTE_INCLUDE_SERVER_PACKAGES"
275275
)
276-
venv.import_package_bundle(system_site_packages_path)
276+
venv.import_package_bundle(system_site_packages_path, priority=True)
277277
venv.import_package_bundle(kernel_site_package_path)
278278

279279
# Phase 7.2: Symlink notebook to jupyter_server for compatibility with

installer/module/virtual_environment.py

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,15 +81,29 @@ def start_server(self, command: str, cwd: Optional[str] = None) -> ServerProcess
8181
return server_proc
8282

8383
def import_package_bundle(
84-
self, bundle_site_package_path: str, condition_env: Optional[str] = None
84+
self,
85+
bundle_site_package_path: str,
86+
condition_env: Optional[str] = None,
87+
*,
88+
priority: bool = False,
8589
) -> None:
8690
"""
8791
Import a package bundle to the virtual environment.
8892
8993
:param bundle_site_package_path: Absolute path to the package bundle.
9094
:param condition_env: Optional environment variable name. If provided, the bundle
91-
will only be loaded when this env var is set to 'true'.
95+
will only be loaded when this env var is set to 'true'.
96+
Cannot be combined with priority.
97+
:param priority: If True, insert at front of sys.path to override other bundles.
98+
Cannot be combined with condition_env.
99+
:raises ValueError: If both condition_env and priority are specified.
92100
"""
101+
if condition_env and priority:
102+
raise ValueError(
103+
"condition_env and priority are mutually exclusive; "
104+
"specify only one of them"
105+
)
106+
93107
pth_file_path = os.path.join(self._site_packages_path, "deepnote.pth")
94108

95109
with open(pth_file_path, "a+", encoding="utf-8") as pth_file:
@@ -105,6 +119,15 @@ def import_package_bundle(
105119
"Bundle was conditionally imported to the virtual environment "
106120
f"(loads when {condition_env}=true)."
107121
)
122+
elif priority:
123+
# Insert at front of sys.path for higher priority (overrides other bundles)
124+
pth_content = (
125+
f"import sys; sys.path.insert(0, '{bundle_site_package_path}')"
126+
)
127+
pth_file.write(pth_content + "\n")
128+
logger.info(
129+
"Bundle was imported with priority to the virtual environment."
130+
)
108131
else:
109132
pth_file.write(bundle_site_package_path + "\n")
110133
logger.info(
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
"""Tests for VirtualEnvironment class."""
2+
3+
from pathlib import Path
4+
from unittest.mock import patch
5+
6+
import pytest
7+
8+
from installer.module.virtual_environment import VirtualEnvironment
9+
10+
11+
class TestImportPackageBundle:
12+
"""Tests for import_package_bundle method."""
13+
14+
@pytest.fixture
15+
def venv(self, tmp_path):
16+
"""Create a VirtualEnvironment with a temporary path."""
17+
venv_path = tmp_path / "venv"
18+
with patch(
19+
"installer.module.virtual_environment.get_current_python_version",
20+
return_value="3.11",
21+
):
22+
venv = VirtualEnvironment(str(venv_path))
23+
# Create the site-packages directory
24+
Path(venv.site_packages_path).mkdir(parents=True, exist_ok=True)
25+
return venv
26+
27+
def test_import_package_bundle_plain_path(self, venv):
28+
"""Test that plain path is appended to .pth file."""
29+
bundle_path = "/some/bundle/site-packages"
30+
31+
venv.import_package_bundle(bundle_path)
32+
33+
pth_file = Path(venv.site_packages_path) / "deepnote.pth"
34+
content = pth_file.read_text()
35+
36+
assert content == f"{bundle_path}\n"
37+
38+
def test_import_package_bundle_with_condition_env(self, venv):
39+
"""Test that conditional import uses sys.path.insert(0, ...)."""
40+
bundle_path = "/server/libs/site-packages"
41+
condition_env = "DEEPNOTE_INCLUDE_SERVER_PACKAGES"
42+
43+
venv.import_package_bundle(bundle_path, condition_env=condition_env)
44+
45+
pth_file = Path(venv.site_packages_path) / "deepnote.pth"
46+
content = pth_file.read_text()
47+
48+
assert "import os, sys" in content
49+
assert f"sys.path.insert(0, '{bundle_path}')" in content
50+
assert f"os.environ.get('{condition_env}', '').lower() == 'true'" in content
51+
52+
def test_import_package_bundle_with_priority(self, venv):
53+
"""Test that priority=True uses sys.path.insert(0, ...)."""
54+
bundle_path = "/usr/local/lib/python3.11/site-packages"
55+
56+
venv.import_package_bundle(bundle_path, priority=True)
57+
58+
pth_file = Path(venv.site_packages_path) / "deepnote.pth"
59+
content = pth_file.read_text()
60+
61+
assert "import sys" in content
62+
assert f"sys.path.insert(0, '{bundle_path}')" in content
63+
assert "os.environ.get" not in content
64+
65+
def test_import_package_bundle_ordering(self, venv):
66+
"""Test .pth file content matches expected order from __main__.py."""
67+
server_libs = "/tmp/python3.11/server-libs/lib/python3.11/site-packages"
68+
system_site = "/usr/local/lib/python3.11/site-packages"
69+
kernel_libs = "/tmp/python3.11/kernel-libs/lib/python3.11/site-packages"
70+
71+
venv.import_package_bundle(
72+
server_libs, condition_env="DEEPNOTE_INCLUDE_SERVER_PACKAGES"
73+
)
74+
venv.import_package_bundle(system_site, priority=True)
75+
venv.import_package_bundle(kernel_libs)
76+
77+
pth_file = Path(venv.site_packages_path) / "deepnote.pth"
78+
lines = pth_file.read_text().splitlines()
79+
80+
assert len(lines) == 3
81+
# Line 1: server-libs conditional
82+
assert "DEEPNOTE_INCLUDE_SERVER_PACKAGES" in lines[0]
83+
assert f"sys.path.insert(0, '{server_libs}')" in lines[0]
84+
# Line 2: system with priority
85+
assert f"sys.path.insert(0, '{system_site}')" in lines[1]
86+
# Line 3: kernel plain path
87+
assert lines[2].strip() == kernel_libs
88+
89+
def test_import_package_bundle_condition_env_and_priority_mutually_exclusive(
90+
self, venv
91+
):
92+
"""Test that condition_env and priority cannot be used together."""
93+
bundle_path = "/some/bundle/site-packages"
94+
95+
with pytest.raises(ValueError, match="mutually exclusive"):
96+
venv.import_package_bundle(
97+
bundle_path,
98+
condition_env="SOME_ENV_VAR",
99+
priority=True,
100+
)

0 commit comments

Comments
 (0)