Skip to content

Commit

Permalink
Load overrides once, rather than in each settings handler (#452)
Browse files Browse the repository at this point in the history
* Load overrides once, rather than in each settings handler

* Add back the overrides loading at handler level as a fallback

To make the change non-breaking in case if other apps
used this handler which is a part of the public API

* Fix tests
  • Loading branch information
krassowski committed May 22, 2024
1 parent 1e0367f commit 348c8ea
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 3 deletions.
9 changes: 9 additions & 0 deletions jupyterlab_server/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
from .licenses_handler import LicensesHandler, LicensesManager
from .listings_handler import ListingsHandler, fetch_listings
from .settings_handler import SettingsHandler
from .settings_utils import _get_overrides
from .themes_handler import ThemesHandler
from .translations_handler import TranslationsHandler
from .workspaces_handler import WorkspacesHandler, WorkspacesManager
Expand Down Expand Up @@ -227,11 +228,19 @@ def add_handlers(handlers: list[Any], extension_app: LabServerApp) -> None:

# Handle local settings.
if extension_app.schemas_dir:
# Load overrides once, rather than in each copy of the settings handler
overrides, error = _get_overrides(extension_app.app_settings_dir)

if error:
overrides_warning = "Failed loading overrides: %s"
extension_app.log.warning(overrides_warning, error)

settings_config: dict[str, Any] = {
"app_settings_dir": extension_app.app_settings_dir,
"schemas_dir": extension_app.schemas_dir,
"settings_dir": extension_app.user_settings_dir,
"labextensions_path": labextensions_path,
"overrides": overrides,
}

# Handle requests for the list of settings. Make slash optional.
Expand Down
3 changes: 2 additions & 1 deletion jupyterlab_server/settings_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,12 @@ def initialize( # type:ignore[override]
schemas_dir: str,
settings_dir: str,
labextensions_path: list[str],
overrides: dict[str, Any] | None = None,
**kwargs: Any, # noqa: ARG002
) -> None:
"""Initialize the handler."""
SchemaHandler.initialize(
self, app_settings_dir, schemas_dir, settings_dir, labextensions_path
self, app_settings_dir, schemas_dir, settings_dir, labextensions_path, overrides
)
ExtensionHandlerMixin.initialize(self, name)

Expand Down
6 changes: 5 additions & 1 deletion jupyterlab_server/settings_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -448,11 +448,15 @@ def initialize(
schemas_dir: str,
settings_dir: str,
labextensions_path: list[str] | None,
overrides: dict[str, Any] | None = None,
**kwargs: Any,
) -> None:
"""Initialize the handler."""
super().initialize(**kwargs)
self.overrides, error = _get_overrides(app_settings_dir)
error = None
if not overrides:
overrides, error = _get_overrides(app_settings_dir)
self.overrides = overrides
self.app_settings_dir = app_settings_dir
self.schemas_dir = schemas_dir
self.settings_dir = settings_dir
Expand Down
8 changes: 7 additions & 1 deletion tests/test_settings_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,13 @@ async def test_get_settings_overrides_dicts(jp_fetch, labserverapp):


@pytest.mark.parametrize("ext", ["json", "json5"])
async def test_get_settings_overrides_d_dicts(jp_fetch, labserverapp, ext):
async def test_get_settings_overrides_d_dicts(
jp_fetch, jp_serverapp, make_labserver_extension_app, ext
):
# Check that values that are dictionaries in overrides.d/*.json are
# merged with the schema.
labserverapp = make_labserver_extension_app()
labserverapp._link_jupyter_server_extension(jp_serverapp)
id = "@jupyterlab/apputils-extension:themes"
overrides_d = Path(labserverapp.app_settings_dir) / "overrides.d"
overrides_d.mkdir(exist_ok=True, parents=True)
Expand All @@ -41,6 +45,8 @@ async def test_get_settings_overrides_d_dicts(jp_fetch, labserverapp, ext):
if ext == "json5":
text += "\n// a comment"
(overrides_d / f"foo-{i}.{ext}").write_text(text, encoding="utf-8")
# Initialize labserverapp only after the overrides were created
labserverapp.initialize()
r = await jp_fetch("lab", "api", "settings", id)
validate_request(r)
res = r.body.decode()
Expand Down

0 comments on commit 348c8ea

Please sign in to comment.