From 6d22cac7227b9e13f742849c75bb6f0ff2515d2c Mon Sep 17 00:00:00 2001 From: Pete <1178561+petetronic@users.noreply.github.com> Date: Wed, 1 May 2024 11:35:47 -0400 Subject: [PATCH] Restore higher limit for listing top level variables in Python (#2912) Also use the higher limit for updates, as more than 100 new variables could be created in an execution. Also increase variables returned for refresh case to use MAX_ITEMS --- .../tests/test_variables.py | 78 +++++++++++++++++++ .../positron/positron_ipykernel/variables.py | 10 +-- 2 files changed, 83 insertions(+), 5 deletions(-) diff --git a/extensions/positron-python/python_files/positron/positron_ipykernel/tests/test_variables.py b/extensions/positron-python/python_files/positron/positron_ipykernel/tests/test_variables.py index 5f029b333c7..3e0454e1b8d 100644 --- a/extensions/positron-python/python_files/positron/positron_ipykernel/tests/test_variables.py +++ b/extensions/positron-python/python_files/positron/positron_ipykernel/tests/test_variables.py @@ -18,6 +18,8 @@ from positron_ipykernel.positron_ipkernel import PositronIPyKernel from positron_ipykernel.utils import JsonRecord, not_none from positron_ipykernel.variables import ( + MAX_CHILDREN, + MAX_ITEMS, VariablesService, _summarize_children, _summarize_variable, @@ -176,6 +178,82 @@ def test_handle_refresh(shell: PositronShell, variables_comm: DummyComm) -> None ] +def test_list_1000(shell: PositronShell, variables_comm: DummyComm) -> None: + # Create 1000 variables + for j in range(0, 1000, 1): + shell.user_ns["var{}".format(j)] = j + + # Request the list of variables + msg = json_rpc_request("list", comm_id="dummy_comm_id") + variables_comm.handle_msg(msg) + + # Assert that a message with 1000 variables is sent + result_msg = variables_comm.messages[0] + assert result_msg.get("data").get("result").get("length") == 1000 + + # Also spot check the first and last variables + variables = result_msg.get("data").get("result").get("variables") + assert variables[0].get("display_name") == "var0" + assert variables[999].get("display_name") == "var999" + + +def test_update_max_children_plus_one(shell: PositronShell, variables_comm: DummyComm) -> None: + # Create and update more than MAX_CHILDREN variables + n = MAX_CHILDREN + 1 + add_value = 500 + msg: Any = create_and_update_n_vars(n, add_value, shell, variables_comm) + + # Check we received an update message + assert msg.get("data").get("method") == "update" + + # Check we did not lose any variables + assigned = msg.get("data").get("params").get("assigned") + assert len(assigned) == n + + # Spot check the first and last variables display values + assert assigned[0].get("display_value") == str(add_value) + assert assigned[n - 1].get("display_value") == str(n - 1 + add_value) + + +def test_update_max_items_plus_one(shell: PositronShell, variables_comm: DummyComm) -> None: + # Create and update more than MAX_ITEMS variables + n = MAX_ITEMS + 1 + add_value = 500 + msg: Any = create_and_update_n_vars(n, add_value, shell, variables_comm) + + # If we exceed MAX_ITEMS, the kernel sends a refresh message instead + assert msg.get("data").get("method") == "refresh" + + # Check we did not exceed MAX_ITEMS variables + variables = msg.get("data").get("params").get("variables") + variables_len = len(variables) + assert variables_len == MAX_ITEMS + + # Spot check the first and last variables display values + assert variables[0].get("display_value") == str(add_value) + assert variables[variables_len - 1].get("display_value") == str(variables_len - 1 + add_value) + + +def create_and_update_n_vars( + n: int, add_value: int, shell: PositronShell, variables_comm: DummyComm +) -> Any: + # Create n variables + assign_n = "" + for j in range(0, n, 1): + assign_n += "x{} = {}".format(j, j) + "\n" + + shell.run_cell(assign_n) + variables_comm.messages.clear() + + # Re-assign the variables to trigger an update message + update_n = "" + for j in range(0, n, 1): + update_n += "x{} = {}".format(j, j + add_value) + "\n" + + shell.run_cell(update_n) + return variables_comm.messages[0] + + @pytest.mark.asyncio async def test_handle_clear( shell: PositronShell, diff --git a/extensions/positron-python/python_files/positron/positron_ipykernel/variables.py b/extensions/positron-python/python_files/positron/positron_ipykernel/variables.py index d5fb75103c6..0d81623353a 100644 --- a/extensions/positron-python/python_files/positron/positron_ipykernel/variables.py +++ b/extensions/positron-python/python_files/positron/positron_ipykernel/variables.py @@ -174,7 +174,7 @@ def _send_update(self, assigned: Mapping[str, Any], removed: Set[str]) -> None: # Filter out hidden assigned variables variables = self._get_filtered_vars(assigned) - filtered_assigned = _summarize_children(variables) + filtered_assigned = _summarize_children(variables, MAX_ITEMS) # Filter out hidden removed variables and encode access keys hidden = self._get_user_ns_hidden() @@ -209,7 +209,7 @@ def send_refresh_event(self) -> None: } """ variables = self._get_filtered_vars() - filtered_variables = _summarize_children(variables) + filtered_variables = _summarize_children(variables, MAX_ITEMS) msg = RefreshParams( variables=filtered_variables, @@ -423,7 +423,7 @@ def _find_var(self, path: Iterable[str]) -> Tuple[bool, Any]: def _list_all_vars(self) -> List[Variable]: variables = self._get_filtered_vars() - return _summarize_children(variables) + return _summarize_children(variables, MAX_ITEMS) def _send_list(self) -> None: filtered_variables = self._list_all_vars() @@ -731,10 +731,10 @@ def _summarize_variable(key: Any, value: Any) -> Optional[Variable]: ) -def _summarize_children(parent: Any) -> List[Variable]: +def _summarize_children(parent: Any, limit: int = MAX_CHILDREN) -> List[Variable]: children = [] for i, (key, value) in enumerate(get_inspector(parent).get_items()): - if i > MAX_CHILDREN: + if len(children) >= limit: break summary = _summarize_variable(key, value) if summary is not None: