Repository: python-lsp/python-lsp-server
Pull request #462 (2023-10-15 21:04:53)

Make workspace/didChangeConfig work with notebook documents
## What change is proposed?

`Notebook` doesn't support `update_config`, as configuration is handled on the cell level. This makes `workspace/didChangeConfguration` error when there are notebook documents in the workspace. This PR fixes that.

## How is this tested?

- [x] unit test
- [x] Manually on Databricks notebooks, which supports notebookDocument messages

tkrabel-db: @ccordoba12 I addressed the comments. Thanks for the swift feedback!

Patch:
diff --git a/pylsp/workspace.py b/pylsp/workspace.py
index 27af5f8..e0e16ef 100644
--- a/pylsp/workspace.py
+++ b/pylsp/workspace.py
@@ -167,7 +167,11 @@ class Workspace:
     def update_config(self, settings):
         self._config.update((settings or {}).get("pylsp", {}))
         for doc_uri in self.documents:
-            self.get_document(doc_uri).update_config(settings)
+            if isinstance(document := self.get_document(doc_uri), Notebook):
+                # Notebook documents don't have a config. The config is
+                # handled at the cell level.
+                return
+            document.update_config(settings)
 
     def apply_edit(self, edit):
         return self._endpoint.request(self.M_APPLY_EDIT, {"edit": edit})


Test patch:
diff --git a/test/test_notebook_document.py b/test/test_notebook_document.py
index 6050b58..15f187d 100644
--- a/test/test_notebook_document.py
+++ b/test/test_notebook_document.py
@@ -2,11 +2,11 @@
 
 import os
 import time
-from unittest.mock import patch, call
 
+from unittest.mock import patch, call
 from test.fixtures import CALL_TIMEOUT_IN_SECONDS
-
 import pytest
+from pylsp.workspace import Notebook
 
 from pylsp import IS_WIN
 from pylsp.lsp import NotebookCellKind
@@ -37,6 +37,59 @@ def test_initialize(client_server_pair):
     assert isinstance(selector, list)
 
 
+@pytest.mark.skipif(IS_WIN, reason="Flaky on Windows")
+def test_workspace_did_change_configuration(client_server_pair):
+    """Test that we can update a workspace config w/o error when a notebook is open."""
+    client, server = client_server_pair
+    client._endpoint.request(
+        "initialize",
+        {
+            "processId": 1234,
+            "rootPath": os.path.dirname(__file__),
+        },
+    ).result(timeout=CALL_TIMEOUT_IN_SECONDS)
+    assert server.workspace is not None
+
+    with patch.object(server._endpoint, "notify") as mock_notify:
+        client._endpoint.notify(
+            "notebookDocument/didOpen",
+            {
+                "notebookDocument": {
+                    "uri": "notebook_uri",
+                    "notebookType": "jupyter-notebook",
+                    "cells": [
+                        {
+                            "kind": NotebookCellKind.Code,
+                            "document": "cell_1_uri",
+                        },
+                    ],
+                },
+                "cellTextDocuments": [
+                    {
+                        "uri": "cell_1_uri",
+                        "languageId": "python",
+                        "text": "",
+                    },
+                ],
+            },
+        )
+        wait_for_condition(lambda: mock_notify.call_count >= 1)
+    assert isinstance(server.workspace.get_document("notebook_uri"), Notebook)
+    assert len(server.workspace.documents) == 2
+
+    server.workspace.update_config(
+        {"pylsp": {"plugins": {"flake8": {"enabled": True}}}}
+    )
+
+    assert server.config.plugin_settings("flake8").get("enabled") is True
+    assert (
+        server.workspace.get_document("cell_1_uri")
+        ._config.plugin_settings("flake8")
+        .get("enabled")
+        is True
+    )
+
+
 @pytest.mark.skipif(IS_WIN, reason="Flaky on Windows")
 def test_notebook_document__did_open(
     client_server_pair,