Skip to content

Commit dadc124

Browse files
committed
fixed subtle bug in extension manager classes—accidently used class attributes instead of instance attributes
1 parent 9ac50db commit dadc124

File tree

12 files changed

+90
-64
lines changed

12 files changed

+90
-64
lines changed

jupyter_server/extension/application.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,7 @@ def _link_jupyter_server_extension(self, serverapp):
322322
# Load config from an ExtensionApp's config files.
323323
self.load_config_file()
324324
# ServerApp's config might have picked up
325-
# CLI config for the ExtensionApp. We call
325+
# config for the ExtensionApp. We call
326326
# update_config to update ExtensionApp's
327327
# traits with these values found in ServerApp's
328328
# config.

jupyter_server/extension/manager.py

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,11 @@ class ExtensionPoint(HasTraits):
2121
point defined by metadata and importable from a Python package.
2222
"""
2323
metadata = Dict()
24-
_app = None
24+
25+
def __init__(self, *args, **kwargs):
26+
# Store extension points that have been linked.
27+
self._app = None
28+
super().__init__(*args, **kwargs)
2529

2630
@validate('metadata')
2731
def _valid_metadata(self, proposed):
@@ -43,10 +47,8 @@ def _valid_metadata(self, proposed):
4347
"sure the extension is installed?".format(self._module_name)
4448
)
4549
# If the metadata includes an ExtensionApp, create an instance.
46-
try:
47-
self._app = metadata.get("app")()
48-
except TypeError:
49-
pass
50+
if 'app' in metadata:
51+
self._app = metadata["app"]()
5052
return metadata
5153

5254
@property
@@ -129,7 +131,11 @@ class ExtensionPackage(HasTraits):
129131
"""
130132
name = Unicode(help="Name of the an importable Python package.")
131133

132-
# Store extension points that have been linked.
134+
def __init__(self, *args, **kwargs):
135+
# Store extension points that have been linked.
136+
self._linked_points = {}
137+
super().__init__(*args, **kwargs)
138+
133139
_linked_points = {}
134140

135141
@validate("name")
@@ -185,23 +191,27 @@ class ExtensionManager(LoggingConfigurable):
185191
Usage:
186192
m = ExtensionManager(jpserver_extensions=extensions)
187193
"""
188-
# The `enabled_extensions` attribute provides a dictionary
189-
# with extension (package) names mapped to their ExtensionPackage interface
190-
# (see above). This manager simplifies the interaction between the
191-
# ServerApp and the extensions being appended.
192-
_enabled_extensions = {}
193-
# The `_linked_extensions` attribute tracks when each extension
194-
# has been successfully linked to a ServerApp. This helps prevent
195-
# extensions from being re-linked recursively unintentionally if another
196-
# extension attempts to link extensions again.
197-
_linked_extensions = {}
194+
def __init__(self, *args, **kwargs):
195+
# The `enabled_extensions` attribute provides a dictionary
196+
# with extension (package) names mapped to their ExtensionPackage interface
197+
# (see above). This manager simplifies the interaction between the
198+
# ServerApp and the extensions being appended.
199+
self._enabled_extensions = {}
200+
# The `_linked_extensions` attribute tracks when each extension
201+
# has been successfully linked to a ServerApp. This helps prevent
202+
# extensions from being re-linked recursively unintentionally if another
203+
# extension attempts to link extensions again.
204+
self._linked_extensions = {}
205+
super().__init__(*args, **kwargs)
198206

199207
@property
200208
def enabled_extensions(self):
201209
"""Dictionary with extension package names as keys
202210
and an ExtensionPackage objects as values.
203211
"""
204-
return dict(sorted(self._enabled_extensions.items()))
212+
# Sort enabled extensions before returning
213+
out = sorted(self._enabled_extensions.items())
214+
return dict(out)
205215

206216
@property
207217
def extension_points(self):
@@ -212,7 +222,6 @@ def extension_points(self):
212222
for name, point in value.extension_points.items()
213223
}
214224

215-
216225
def from_jpserver_extensions(self, jpserver_extensions):
217226
"""Add extensions from 'jpserver_extensions'-like dictionary."""
218227
for name, enabled in jpserver_extensions.items():

jupyter_server/pytest_plugin.py

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -111,16 +111,18 @@ def extension_environ(env_config_path, monkeypatch):
111111
@pytest.fixture(scope='function')
112112
def configurable_serverapp(
113113
environ,
114+
server_config,
115+
argv,
114116
http_port,
115117
tmp_path,
116118
root_dir,
117119
io_loop,
118-
server_config,
119-
**kwargs
120120
):
121-
def serverapp(
121+
ServerApp.clear_instance()
122+
123+
def _configurable_serverapp(
122124
config=server_config,
123-
argv=[],
125+
argv=argv,
124126
environ=environ,
125127
http_port=http_port,
126128
tmp_path=tmp_path,
@@ -144,6 +146,7 @@ def serverapp(
144146
token=token,
145147
**kwargs
146148
)
149+
147150
app.init_signal = lambda: None
148151
app.log.propagate = True
149152
app.log.handlers = []
@@ -155,12 +158,11 @@ def serverapp(
155158
app.start_app()
156159
return app
157160

158-
yield serverapp
159-
ServerApp.clear_instance()
161+
return _configurable_serverapp
160162

161163

162-
@pytest.fixture
163-
def serverapp(configurable_serverapp, server_config, argv):
164+
@pytest.fixture(scope="function")
165+
def serverapp(server_config, argv, configurable_serverapp):
164166
app = configurable_serverapp(config=server_config, argv=argv)
165167
yield app
166168
app.remove_server_info_file()

jupyter_server/serverapp.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1503,7 +1503,7 @@ def init_server_extensions(self):
15031503
and load its own config.
15041504
"""
15051505
# Create an instance of the ExtensionManager.
1506-
self.extension_manager = ExtensionManager(logger=self.log)
1506+
self.extension_manager = ExtensionManager(log=self.log)
15071507
self.extension_manager.from_jpserver_extensions(self.jpserver_extensions)
15081508
self.extension_manager.link_all_extensions(self)
15091509

@@ -1682,6 +1682,7 @@ def initialize(self, argv=None, find_extensions=True, new_httpserver=True):
16821682
self.init_mime_overrides()
16831683
self.init_shutdown_no_activity()
16841684

1685+
16851686
def cleanup_kernels(self):
16861687
"""Shutdown all kernels.
16871688

tests/conftest.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
pytest_plugins = ['pytest_jupyter_server']
1+
pytest_plugins = ['pytest_jupyter_server']

tests/extension/conftest.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import pytest
22

3+
34
mock_html = """
45
<!DOCTYPE HTML>
56
<html>
@@ -23,6 +24,7 @@
2324
"""
2425

2526

27+
2628
@pytest.fixture
2729
def mock_template(template_dir):
2830
index = template_dir.joinpath('index.html')

tests/extension/test_app.py

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,9 @@
11
import pytest
22
from jupyter_server.serverapp import ServerApp
33

4-
# Use ServerApps environment because it monkeypatches
5-
# jupyter_core.paths and provides a config directory
6-
# that's not cross contaminating the user config directory.
7-
pytestmark = pytest.mark.usefixtures("environ")
8-
94

105
@pytest.fixture
11-
def server_config(request, template_dir):
6+
def server_config(template_dir):
127
config = {
138
"ServerApp": {
149
"jpserver_extensions": {
@@ -26,15 +21,15 @@ def server_config(request, template_dir):
2621

2722

2823
@pytest.fixture
29-
def mock_extension(serverapp, extension_manager):
24+
def mock_extension(extension_manager):
3025
name = "tests.extension.mockextensions"
3126
pkg = extension_manager.enabled_extensions[name]
3227
point = pkg.extension_points["mockextension"]
3328
app = point.app
3429
return app
3530

3631

37-
def test_initialize(serverapp, mock_extension, template_dir):
32+
def test_initialize(serverapp, template_dir, mock_extension):
3833
# Check that settings and handlers were added to the mock extension.
3934
assert isinstance(mock_extension.serverapp, ServerApp)
4035
assert len(mock_extension.handlers) > 0
@@ -53,7 +48,6 @@ def test_initialize(serverapp, mock_extension, template_dir):
5348
)
5449
)
5550
def test_instance_creation_with_argv(
56-
serverapp,
5751
trait_name,
5852
trait_value,
5953
mock_extension,
@@ -62,10 +56,9 @@ def test_instance_creation_with_argv(
6256

6357

6458
def test_extensionapp_load_config_file(
65-
extension_environ,
6659
config_file,
67-
mock_extension,
6860
serverapp,
61+
mock_extension,
6962
):
7063
# Assert default config_file_paths is the same in the app and extension.
7164
assert mock_extension.config_file_paths == serverapp.config_file_paths

tests/extension/test_manager.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@
77
ExtensionModuleNotFound
88
)
99

10+
# Use ServerApps environment because it monkeypatches
11+
# jupyter_core.paths and provides a config directory
12+
# that's not cross contaminating the user config directory.
13+
pytestmark = pytest.mark.usefixtures("environ")
14+
1015

1116
def test_extension_point_api():
1217
# Import mock extension metadata

tests/extension/test_serverextension.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,12 @@
99
from jupyter_server.config_manager import BaseJSONConfigManager
1010

1111

12+
# Use ServerApps environment because it monkeypatches
13+
# jupyter_core.paths and provides a config directory
14+
# that's not cross contaminating the user config directory.
15+
pytestmark = pytest.mark.usefixtures("environ")
16+
17+
1218
def test_help_output():
1319
check_help_all_output('jupyter_server.extension.serverextension')
1420
check_help_all_output('jupyter_server.extension.serverextension', ['enable'])

tests/extension/test_utils.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,13 @@
22
from jupyter_server.extension.utils import validate_extension
33

44

5+
# Use ServerApps environment because it monkeypatches
6+
# jupyter_core.paths and provides a config directory
7+
# that's not cross contaminating the user config directory.
8+
pytestmark = pytest.mark.usefixtures("environ")
9+
10+
11+
512
def test_validate_extension():
613
# enabled at sys level
714
assert validate_extension('tests.extension.mockextensions.mockext_sys')

0 commit comments

Comments
 (0)