From a601d46339415eda72bd299ea7291f9433fef09e Mon Sep 17 00:00:00 2001 From: Quentin Peter Date: Thu, 23 May 2024 21:00:49 +0200 Subject: [PATCH 01/20] SpyderFrame --- spyder_kernels/console/kernel.py | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/spyder_kernels/console/kernel.py b/spyder_kernels/console/kernel.py index 15a407aa..d626f1bd 100644 --- a/spyder_kernels/console/kernel.py +++ b/spyder_kernels/console/kernel.py @@ -11,6 +11,7 @@ """ # Standard library imports +from collections import namedtuple import faulthandler import json import logging @@ -261,6 +262,10 @@ def get_current_frames(self, ignore_internal_threads=True): thread_names = {thread.ident: thread.name for thread in threading.enumerate()} + SpyderFrameSummary = namedtuple( + "SpyderFrameSummary", ["filename", "lineno", "name", "line"] + ) + for thread_id, frame in sys._current_frames().items(): stack = traceback.StackSummary.extract( traceback.walk_stack(frame)) @@ -274,7 +279,17 @@ def get_current_frames(self, ignore_internal_threads=True): thread_name = thread_names[thread_id] else: thread_name = str(thread_id) - frames[thread_name] = stack + # Transform stack in a named tuple because FrameSummary objects + # are not compatible between versions of python + frames[thread_name] = [ + SpyderFrameSummary( + frame.filename, + frame.lineno, + frame.name, + frame.line + ) + for frame in stack + ] return frames # --- For the Variable Explorer @@ -705,7 +720,7 @@ def set_special_kernel(self, special): exec("from pylab import *", self.shell.user_ns) self.shell.special = special return - + if special == "sympy": import sympy # noqa sympy_init = "\n".join([ From 3448ea1550af760224ba761dc3bb12f3319fb86e Mon Sep 17 00:00:00 2001 From: Quentin Peter Date: Sat, 25 May 2024 12:55:48 +0200 Subject: [PATCH 02/20] class --- spyder_kernels/console/kernel.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/spyder_kernels/console/kernel.py b/spyder_kernels/console/kernel.py index d626f1bd..8bfe534b 100644 --- a/spyder_kernels/console/kernel.py +++ b/spyder_kernels/console/kernel.py @@ -11,7 +11,6 @@ """ # Standard library imports -from collections import namedtuple import faulthandler import json import logging @@ -50,6 +49,14 @@ EXCLUDED_NAMES = ['In', 'Out', 'exit', 'get_ipython', 'quit'] +class SpyderFrameSummary(): + def __init__(self, filename, lineno, name, line): + self.filename = filename + self.lineno = lineno + self.name = name + self.line = line + + class SpyderKernel(IPythonKernel): """Spyder kernel for Jupyter.""" @@ -262,10 +269,6 @@ def get_current_frames(self, ignore_internal_threads=True): thread_names = {thread.ident: thread.name for thread in threading.enumerate()} - SpyderFrameSummary = namedtuple( - "SpyderFrameSummary", ["filename", "lineno", "name", "line"] - ) - for thread_id, frame in sys._current_frames().items(): stack = traceback.StackSummary.extract( traceback.walk_stack(frame)) From 8edb4c210abf7431fb53a69d0461bd9e5bf43192 Mon Sep 17 00:00:00 2001 From: Quentin Peter Date: Sat, 25 May 2024 17:41:07 +0200 Subject: [PATCH 03/20] Use json instead of cloudpickle --- spyder_kernels/comms/commbase.py | 76 +++++++++---------- spyder_kernels/comms/frontendcomm.py | 2 - spyder_kernels/console/kernel.py | 20 ++--- .../console/tests/test_console_kernel.py | 6 +- spyder_kernels/customize/namespace_manager.py | 1 - spyder_kernels/customize/spyderpdb.py | 12 +++ 6 files changed, 55 insertions(+), 62 deletions(-) diff --git a/spyder_kernels/comms/commbase.py b/spyder_kernels/comms/commbase.py index 20e3a8a3..16a65a7b 100644 --- a/spyder_kernels/comms/commbase.py +++ b/spyder_kernels/comms/commbase.py @@ -8,7 +8,7 @@ Class that handles communications between Spyder kernel and frontend. Comms transmit data in a list of buffers, and in a json-able dictionnary. -Here, we only support a buffer list with a single element. +Here, we only support json. The messages exchanged have the following msg_dict: @@ -19,7 +19,7 @@ } ``` -The buffer is generated by cloudpickle using `PICKLE_PROTOCOL = 2`. +The buffer is generated by json. To simplify the usage of messaging, we use a higher level function calling mechanism: @@ -50,8 +50,6 @@ 'call_name': The function name (mostly for debugging) } """ -import cloudpickle -import pickle import logging import sys import uuid @@ -60,8 +58,6 @@ logger = logging.getLogger(__name__) -DEFAULT_PICKLE_PROTOCOL = 4 - # Max timeout (in secs) for blocking calls TIMEOUT = 3 @@ -76,6 +72,28 @@ def __init__(self, call_name, call_id): self.call_id = call_id self.etype, self.error, tb = sys.exc_info() self.tb = traceback.extract_tb(tb) + + def to_json(self): + return { + "call_name": self.call_name, + "call_id": self.call_id, + "etype": self.etype.__name__, + "args": self.error.args, + "tb": [ + (frame.filename, frame.lineno, frame.name, frame.line) + for frame in self.tb + ] + } + + @classmethod + def from_json(cls, json_data): + instance = cls.__new__(cls) + instance.call_name = json_data["call_name"] + instance.call_id = json_data["call_id"] + instance.etype = type(json_data["etype"], (Exception,), {}) + instance.error = instance.etype(json_data["args"]) + instance.tb = traceback.StackSummary.from_list(json_data["tb"]) + return instance def raise_error(self): """ @@ -216,8 +234,7 @@ def _send_message(self, spyder_msg_type, content=None, data=None, content: dict The (JSONable) content of the message data: any - Any object that is serializable by cloudpickle (should be most - things). Will arrive as cloudpickled bytes in `.buffers[0]`. + Any object that is serializable by json. comm_id: int the comm to send to. If None sends to all comms. """ @@ -228,17 +245,11 @@ def _send_message(self, spyder_msg_type, content=None, data=None, msg_dict = { 'spyder_msg_type': spyder_msg_type, 'content': content, - 'pickle_protocol': self._comms[comm_id]['pickle_protocol'], 'python_version': sys.version, + 'data': data } - buffers = [cloudpickle.dumps( - data, protocol=self._comms[comm_id]['pickle_protocol'])] - self._comms[comm_id]['comm'].send(msg_dict, buffers=buffers) - def _set_pickle_protocol(self, protocol): - """Set the pickle protocol used to send data.""" - protocol = min(protocol, pickle.HIGHEST_PROTOCOL) - self._comms[self.calling_comm_id]['pickle_protocol'] = protocol + self._comms[comm_id]['comm'].send(msg_dict) @property def _comm_name(self): @@ -275,7 +286,6 @@ def _register_comm(self, comm): comm.on_close(self._comm_close) self._comms[comm.comm_id] = { 'comm': comm, - 'pickle_protocol': DEFAULT_PICKLE_PROTOCOL, 'status': 'opening', } @@ -292,19 +302,7 @@ def _comm_message(self, msg): # Get message dict msg_dict = msg['content']['data'] - - # Load the buffer. Only one is supported. - try: - buffer = cloudpickle.loads(msg['buffers'][0]) - except Exception as e: - logger.debug( - "Exception in cloudpickle.loads : %s" % str(e)) - buffer = CommsErrorWrapper( - msg_dict['content']['call_name'], - msg_dict['content']['call_id']) - - msg_dict['content']['is_error'] = True - + buffer = msg_dict["data"] spyder_msg_type = msg_dict['spyder_msg_type'] if spyder_msg_type in self._message_handlers: @@ -317,10 +315,6 @@ def _handle_remote_call(self, msg, buffer): """Handle a remote call.""" msg_dict = msg['content'] self.on_incoming_call(msg_dict) - if msg['content'].get('is_error', False): - # could not open the pickle - self._set_call_return_value(msg, buffer, is_error=True) - return try: return_value = self._remote_callback( msg_dict['call_name'], @@ -350,8 +344,10 @@ def _set_call_return_value(self, call_dict, data, is_error=False): display_error = ('display_error' in settings and settings['display_error']) - if is_error and display_error: - data.print_error() + if is_error: + if display_error: + data.print_error() + data = data.to_json() send_reply = 'send_reply' in settings and settings['send_reply'] if not send_reply: @@ -378,13 +374,11 @@ def _register_call(self, call_dict, callback=None): def on_outgoing_call(self, call_dict): """A message is about to be sent""" - call_dict["pickle_highest_protocol"] = pickle.HIGHEST_PROTOCOL return call_dict def on_incoming_call(self, call_dict): """A call was received""" - if "pickle_highest_protocol" in call_dict: - self._set_pickle_protocol(call_dict["pickle_highest_protocol"]) + pass def _send_call(self, call_dict, call_data, comm_id): """Send call.""" @@ -471,13 +465,13 @@ def _async_error(self, error_wrapper): """ Handle an error that was raised on the other side asyncronously. """ - error_wrapper.print_error() + CommsErrorWrapper.from_json(error_wrapper).print_error() def _sync_error(self, error_wrapper): """ Handle an error that was raised on the other side syncronously. """ - error_wrapper.raise_error() + CommsErrorWrapper.from_json(error_wrapper).raise_error() class RemoteCallFactory: diff --git a/spyder_kernels/comms/frontendcomm.py b/spyder_kernels/comms/frontendcomm.py index 9eaf2ee1..dd644b2e 100644 --- a/spyder_kernels/comms/frontendcomm.py +++ b/spyder_kernels/comms/frontendcomm.py @@ -176,8 +176,6 @@ def _comm_open(self, comm, msg): """ self.calling_comm_id = comm.comm_id self._register_comm(comm) - self._set_pickle_protocol( - msg['content']['data']['pickle_highest_protocol']) # IOPub might not be connected yet, keep sending messages until a # reply is received. diff --git a/spyder_kernels/console/kernel.py b/spyder_kernels/console/kernel.py index 8bfe534b..42714d6d 100644 --- a/spyder_kernels/console/kernel.py +++ b/spyder_kernels/console/kernel.py @@ -49,14 +49,6 @@ EXCLUDED_NAMES = ['In', 'Out', 'exit', 'get_ipython', 'quit'] -class SpyderFrameSummary(): - def __init__(self, filename, lineno, name, line): - self.filename = filename - self.lineno = lineno - self.name = name - self.line = line - - class SpyderKernel(IPythonKernel): """Spyder kernel for Jupyter.""" @@ -285,12 +277,12 @@ def get_current_frames(self, ignore_internal_threads=True): # Transform stack in a named tuple because FrameSummary objects # are not compatible between versions of python frames[thread_name] = [ - SpyderFrameSummary( - frame.filename, - frame.lineno, - frame.name, - frame.line - ) + { + "filename": frame.filename, + "lineno": frame.lineno, + "name": frame.name, + "line": frame.line + } for frame in stack ] return frames diff --git a/spyder_kernels/console/tests/test_console_kernel.py b/spyder_kernels/console/tests/test_console_kernel.py index b7bbbc6b..478b986b 100644 --- a/spyder_kernels/console/tests/test_console_kernel.py +++ b/spyder_kernels/console/tests/test_console_kernel.py @@ -1272,13 +1272,12 @@ def test_interrupt(): """ # Command to start the kernel cmd = "from spyder_kernels.console import start; start.main()" - import pickle with setup_kernel(cmd) as client: kernel_comm = CommBase() # Create new comm and send the highest protocol comm = Comm(kernel_comm._comm_name, client) - comm.open(data={'pickle_highest_protocol': pickle.HIGHEST_PROTOCOL}) + comm.open(data={}) comm._send_channel = client.control_channel kernel_comm._register_comm(comm) @@ -1328,13 +1327,12 @@ def test_enter_debug_after_interruption(): """ # Command to start the kernel cmd = "from spyder_kernels.console import start; start.main()" - import pickle with setup_kernel(cmd) as client: kernel_comm = CommBase() # Create new comm and send the highest protocol comm = Comm(kernel_comm._comm_name, client) - comm.open(data={'pickle_highest_protocol': pickle.HIGHEST_PROTOCOL}) + comm.open(data={}) comm._send_channel = client.control_channel kernel_comm._register_comm(comm) diff --git a/spyder_kernels/customize/namespace_manager.py b/spyder_kernels/customize/namespace_manager.py index f758cf27..c3fcbcba 100755 --- a/spyder_kernels/customize/namespace_manager.py +++ b/spyder_kernels/customize/namespace_manager.py @@ -76,7 +76,6 @@ def __enter__(self): self.ns_globals = main_mod.__dict__ self.ns_locals = None - # Needed to allow pickle to reference main if '__main__' in sys.modules: self._previous_main = sys.modules['__main__'] sys.modules['__main__'] = main_mod diff --git a/spyder_kernels/customize/spyderpdb.py b/spyder_kernels/customize/spyderpdb.py index 8fd4e975..cffa035c 100755 --- a/spyder_kernels/customize/spyderpdb.py +++ b/spyder_kernels/customize/spyderpdb.py @@ -830,6 +830,18 @@ def get_pdb_state(self): if self.pdb_publish_stack: # Publish Pdb stack so we can update the Debugger plugin on Spyder pdb_stack = traceback.StackSummary.extract(self.stack) + + pdb_stack = [ + { + "filename": frame.filename, + "lineno": frame.lineno, + "name": frame.name, + "line": frame.line + } + for frame in pdb_stack + ] + + pdb_index = self.curindex skip_hidden = getattr(self, 'skip_hidden', False) From 4cc697fc8acffcdfe9f2bbc6c3170259265b5c92 Mon Sep 17 00:00:00 2001 From: Quentin Peter Date: Sat, 25 May 2024 19:09:23 +0200 Subject: [PATCH 04/20] etype from builtins --- spyder_kernels/comms/commbase.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/spyder_kernels/comms/commbase.py b/spyder_kernels/comms/commbase.py index 16a65a7b..4dd5ad82 100644 --- a/spyder_kernels/comms/commbase.py +++ b/spyder_kernels/comms/commbase.py @@ -54,6 +54,7 @@ import sys import uuid import traceback +import builtins logger = logging.getLogger(__name__) @@ -90,7 +91,12 @@ def from_json(cls, json_data): instance = cls.__new__(cls) instance.call_name = json_data["call_name"] instance.call_id = json_data["call_id"] - instance.etype = type(json_data["etype"], (Exception,), {}) + etype = json_data["etype"] + instance.etype = getattr( + builtins, + etype, + type(etype, (Exception,), {}) + ) instance.error = instance.etype(json_data["args"]) instance.tb = traceback.StackSummary.from_list(json_data["tb"]) return instance From 4f41ea9f61e64c3241da294fe8b3427f11ade359 Mon Sep 17 00:00:00 2001 From: Quentin Peter Date: Sat, 25 May 2024 19:12:39 +0200 Subject: [PATCH 05/20] trailing spaces --- spyder_kernels/comms/commbase.py | 10 +++++----- spyder_kernels/console/tests/test_console_kernel.py | 6 +++--- spyder_kernels/customize/spyderpdb.py | 8 ++++---- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/spyder_kernels/comms/commbase.py b/spyder_kernels/comms/commbase.py index 4dd5ad82..1857f560 100644 --- a/spyder_kernels/comms/commbase.py +++ b/spyder_kernels/comms/commbase.py @@ -73,7 +73,7 @@ def __init__(self, call_name, call_id): self.call_id = call_id self.etype, self.error, tb = sys.exc_info() self.tb = traceback.extract_tb(tb) - + def to_json(self): return { "call_name": self.call_name, @@ -81,11 +81,11 @@ def to_json(self): "etype": self.etype.__name__, "args": self.error.args, "tb": [ - (frame.filename, frame.lineno, frame.name, frame.line) + (frame.filename, frame.lineno, frame.name, frame.line) for frame in self.tb ] } - + @classmethod def from_json(cls, json_data): instance = cls.__new__(cls) @@ -93,8 +93,8 @@ def from_json(cls, json_data): instance.call_id = json_data["call_id"] etype = json_data["etype"] instance.etype = getattr( - builtins, - etype, + builtins, + etype, type(etype, (Exception,), {}) ) instance.error = instance.etype(json_data["args"]) diff --git a/spyder_kernels/console/tests/test_console_kernel.py b/spyder_kernels/console/tests/test_console_kernel.py index 478b986b..60f240d3 100644 --- a/spyder_kernels/console/tests/test_console_kernel.py +++ b/spyder_kernels/console/tests/test_console_kernel.py @@ -1391,7 +1391,7 @@ def test_django_settings(kernel): def test_hard_link_pdb(tmpdir): """ - Test that breakpoints on a file are recognised even when the path is + Test that breakpoints on a file are recognised even when the path is different. """ # Create a file and a hard link @@ -1401,10 +1401,10 @@ def test_hard_link_pdb(tmpdir): os.mkdir(folder) hard_link = folder.join("file.py") os.link(d, hard_link) - + # Make sure both paths point to the same file assert os.path.samefile(d, hard_link) - + # Make sure canonic returns the same path for a single file pdb_obj = SpyderPdb() assert pdb_obj.canonic(str(d)) == pdb_obj.canonic(str(hard_link)) diff --git a/spyder_kernels/customize/spyderpdb.py b/spyder_kernels/customize/spyderpdb.py index cffa035c..4bdee99a 100755 --- a/spyder_kernels/customize/spyderpdb.py +++ b/spyder_kernels/customize/spyderpdb.py @@ -830,7 +830,7 @@ def get_pdb_state(self): if self.pdb_publish_stack: # Publish Pdb stack so we can update the Debugger plugin on Spyder pdb_stack = traceback.StackSummary.extract(self.stack) - + pdb_stack = [ { "filename": frame.filename, @@ -839,9 +839,9 @@ def get_pdb_state(self): "line": frame.line } for frame in pdb_stack - ] - - + ] + + pdb_index = self.curindex skip_hidden = getattr(self, 'skip_hidden', False) From e04ff7911fd61927f737b7eaa2e176fa20325d00 Mon Sep 17 00:00:00 2001 From: Quentin Peter Date: Sat, 25 May 2024 22:58:46 +0200 Subject: [PATCH 06/20] traceback to json --- spyder_kernels/console/shell.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/spyder_kernels/console/shell.py b/spyder_kernels/console/shell.py index 48cecaf3..88a1162e 100644 --- a/spyder_kernels/console/shell.py +++ b/spyder_kernels/console/shell.py @@ -271,7 +271,18 @@ def showtraceback(self, exc_tuple=None, filename=None, tb_offset=None, if not exception_only: try: etype, value, tb = self._get_exc_info(exc_tuple) + etype = etype.__name__ + value = value.args stack = traceback.extract_tb(tb.tb_next) + stack = [ + { + "filename": frame.filename, + "lineno": frame.lineno, + "name": frame.name, + "line": frame.line + } + for frame in stack + ] self.kernel.frontend_call(blocking=False).show_traceback( etype, value, stack) except Exception: From 5382aa6c451f9919ba585b57dbdf580442e52138 Mon Sep 17 00:00:00 2001 From: Quentin Peter Date: Sat, 25 May 2024 23:00:04 +0200 Subject: [PATCH 07/20] remove cloudpickle --- README.md | 1 - requirements/posix.txt | 1 - requirements/windows.txt | 1 - setup.py | 1 - 4 files changed, 4 deletions(-) diff --git a/README.md b/README.md index b6307c10..686bfd7a 100644 --- a/README.md +++ b/README.md @@ -41,7 +41,6 @@ pip install spyder-kernels This project depends on: * [ipykernel](https://github.com/ipython/ipykernel/) -* [cloudpickle](https://github.com/cloudpipe/cloudpickle) * [wurlitzer](https://github.com/minrk/wurlitzer) (only on Linux and macOS). ## Changelog diff --git a/requirements/posix.txt b/requirements/posix.txt index 0c22fa57..cd73281e 100644 --- a/requirements/posix.txt +++ b/requirements/posix.txt @@ -1,4 +1,3 @@ -cloudpickle ipykernel>=6.29.3,<7 ipython>=8.12.2,<9 jupyter_client>=7.4.9,<9 diff --git a/requirements/windows.txt b/requirements/windows.txt index 9d6370b6..6289ad3e 100644 --- a/requirements/windows.txt +++ b/requirements/windows.txt @@ -1,4 +1,3 @@ -cloudpickle ipykernel>=6.29.3,<7 ipython>=8.12.2,<9 jupyter_client>=7.4.9,<9 diff --git a/setup.py b/setup.py index 79689b46..5e0b0f8f 100644 --- a/setup.py +++ b/setup.py @@ -36,7 +36,6 @@ def get_version(module='spyder_kernels'): REQUIREMENTS = [ - 'cloudpickle', 'ipykernel>=6.29.3,<7', 'ipython>=8.12.2,<8.13; python_version=="3.8"', 'ipython>=8.13.0,<9,!=8.17.1; python_version>"3.8"', From d04b8e27d29eee9e918577917e4f0017bbb503f5 Mon Sep 17 00:00:00 2001 From: Quentin Peter Date: Sun, 26 May 2024 10:59:52 +0200 Subject: [PATCH 08/20] remove buffer --- spyder_kernels/comms/commbase.py | 70 +++++++++++++++----------------- 1 file changed, 32 insertions(+), 38 deletions(-) diff --git a/spyder_kernels/comms/commbase.py b/spyder_kernels/comms/commbase.py index 1857f560..5c8627b8 100644 --- a/spyder_kernels/comms/commbase.py +++ b/spyder_kernels/comms/commbase.py @@ -8,7 +8,8 @@ Class that handles communications between Spyder kernel and frontend. Comms transmit data in a list of buffers, and in a json-able dictionnary. -Here, we only support json. +Here, we only support json to avoid issues of compatibility between python +versions. The messages exchanged have the following msg_dict: @@ -16,11 +17,10 @@ msg_dict = { 'spyder_msg_type': spyder_msg_type, 'content': content, - } + 'python_version': sys.version, + } ``` -The buffer is generated by json. - To simplify the usage of messaging, we use a higher level function calling mechanism: - The `remote_call` method returns a RemoteCallHandler object @@ -35,19 +35,17 @@ 'call_name': The name of the function to be called, 'call_id': uuid to match the request to a potential reply, 'settings': A dictionnary of settings, - } - - The buffer encodes a dictionnary { 'call_args': The function args, 'call_kwargs': The function kwargs, } - If the 'settings' has `'blocking' = True`, a reply is sent. (spyder_msg_type = 'remote_call_reply'): - - The buffer contains the return value of the function. - The 'content' is a dict with: { 'is_error': a boolean indicating if the return value is an exception to be raised. 'call_id': The uuid from above, - 'call_name': The function name (mostly for debugging) + 'call_name': The function name (mostly for debugging), + 'call_return_value': The return value of the function } """ import logging @@ -228,8 +226,7 @@ def remote_call(self, comm_id=None, callback=None, **settings): return RemoteCallFactory(self, comm_id, callback, **settings) # ---- Private ----- - def _send_message(self, spyder_msg_type, content=None, data=None, - comm_id=None): + def _send_message(self, spyder_msg_type, content=None, comm_id=None): """ Publish custom messages to the other side. @@ -252,7 +249,6 @@ def _send_message(self, spyder_msg_type, content=None, data=None, 'spyder_msg_type': spyder_msg_type, 'content': content, 'python_version': sys.version, - 'data': data } self._comms[comm_id]['comm'].send(msg_dict) @@ -273,9 +269,8 @@ def _register_message_handler(self, message_id, handler): message_id : str The identifier for the message handler : callback - A function to handle the message. This is called with 3 arguments: + A function to handle the message. This is called with: - msg_dict: A dictionary with message information. - - buffer: The data transmitted in the buffer Pass None to unregister the message_id """ if handler is None: @@ -308,24 +303,22 @@ def _comm_message(self, msg): # Get message dict msg_dict = msg['content']['data'] - buffer = msg_dict["data"] spyder_msg_type = msg_dict['spyder_msg_type'] if spyder_msg_type in self._message_handlers: - self._message_handlers[spyder_msg_type]( - msg_dict, buffer) + self._message_handlers[spyder_msg_type](msg_dict) else: logger.debug("No such spyder message type: %s" % spyder_msg_type) - def _handle_remote_call(self, msg, buffer): + def _handle_remote_call(self, msg): """Handle a remote call.""" msg_dict = msg['content'] self.on_incoming_call(msg_dict) try: return_value = self._remote_callback( msg_dict['call_name'], - buffer['call_args'], - buffer['call_kwargs']) + msg_dict['call_args'], + msg_dict['call_kwargs']) self._set_call_return_value(msg_dict, return_value) except Exception: exc_infos = CommsErrorWrapper( @@ -340,7 +333,7 @@ def _remote_callback(self, call_name, call_args, call_kwargs): raise CommError("No such spyder call type: %s" % call_name) - def _set_call_return_value(self, call_dict, data, is_error=False): + def _set_call_return_value(self, call_dict, return_value, is_error=False): """ A remote call has just been processed. @@ -352,8 +345,8 @@ def _set_call_return_value(self, call_dict, data, is_error=False): settings['display_error']) if is_error: if display_error: - data.print_error() - data = data.to_json() + return_value.print_error() + return_value = return_value.to_json() send_reply = 'send_reply' in settings and settings['send_reply'] if not send_reply: @@ -362,11 +355,13 @@ def _set_call_return_value(self, call_dict, data, is_error=False): content = { 'is_error': is_error, 'call_id': call_dict['call_id'], - 'call_name': call_dict['call_name'] + 'call_name': call_dict['call_name'], + 'call_return_value': return_value } - self._send_message('remote_call_reply', content=content, data=data, - comm_id=self.calling_comm_id) + self._send_message( + 'remote_call_reply', content=content, comm_id=self.calling_comm_id + ) def _register_call(self, call_dict, callback=None): """ @@ -386,12 +381,12 @@ def on_incoming_call(self, call_dict): """A call was received""" pass - def _send_call(self, call_dict, call_data, comm_id): + def _send_call(self, call_dict, comm_id): """Send call.""" call_dict = self.on_outgoing_call(call_dict) self._send_message( - 'remote_call', content=call_dict, data=call_data, - comm_id=comm_id) + 'remote_call', content=call_dict, comm_id=comm_id + ) def _get_call_return_value(self, call_dict, comm_id): """ @@ -419,11 +414,13 @@ def _get_call_return_value(self, call_dict, comm_id): self._wait_reply(comm_id, call_id, call_name, timeout) reply = self._reply_inbox.pop(call_id) + + value = reply['content']['call_return_value'] if reply['is_error']: - return self._sync_error(reply['value']) + return self._sync_error(value) - return reply['value'] + return value def _wait_reply(self, comm_id, call_id, call_name, timeout): """ @@ -431,7 +428,7 @@ def _wait_reply(self, comm_id, call_id, call_name, timeout): """ raise NotImplementedError - def _handle_remote_call_reply(self, msg_dict, buffer): + def _handle_remote_call_reply(self, msg_dict): """ A blocking call received a reply. """ @@ -443,7 +440,7 @@ def _handle_remote_call_reply(self, msg_dict, buffer): # Unexpected reply if call_id not in self._reply_waitlist: if is_error: - return self._async_error(buffer) + return self._async_error(msg_dict['call_return_value']) else: logger.debug('Got an unexpected reply {}, id:{}'.format( call_name, call_id)) @@ -453,17 +450,16 @@ def _handle_remote_call_reply(self, msg_dict, buffer): # Async error if is_error and not blocking: - return self._async_error(buffer) + return self._async_error(content['call_return_value']) # Callback if callback is not None and not is_error: - callback(buffer) + callback(content['call_return_value']) # Blocking inbox if blocking: self._reply_inbox[call_id] = { 'is_error': is_error, - 'value': buffer, 'content': content } @@ -525,8 +521,6 @@ def __call__(self, *args, **kwargs): 'call_name': self._name, 'call_id': call_id, 'settings': self._settings, - } - call_data = { 'call_args': args, 'call_kwargs': kwargs, } @@ -538,6 +532,6 @@ def __call__(self, *args, **kwargs): logger.debug("Call to unconnected comm: %s" % self._name) return self._comms_wrapper._register_call(call_dict, self._callback) - self._comms_wrapper._send_call(call_dict, call_data, self._comm_id) + self._comms_wrapper._send_call(call_dict, self._comm_id) return self._comms_wrapper._get_call_return_value( call_dict, self._comm_id) From c3b94357e06d16c394da8174ae1239177212cd17 Mon Sep 17 00:00:00 2001 From: Quentin Peter Date: Sun, 26 May 2024 11:17:52 +0200 Subject: [PATCH 09/20] whitespace --- spyder_kernels/comms/commbase.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spyder_kernels/comms/commbase.py b/spyder_kernels/comms/commbase.py index 5c8627b8..27a6dfa9 100644 --- a/spyder_kernels/comms/commbase.py +++ b/spyder_kernels/comms/commbase.py @@ -414,7 +414,7 @@ def _get_call_return_value(self, call_dict, comm_id): self._wait_reply(comm_id, call_id, call_name, timeout) reply = self._reply_inbox.pop(call_id) - + value = reply['content']['call_return_value'] if reply['is_error']: From ab7ce7c6d36636f54ef8de9c95da49f7ddac9bcd Mon Sep 17 00:00:00 2001 From: Quentin Peter Date: Sun, 26 May 2024 13:05:18 +0200 Subject: [PATCH 10/20] cleanup --- spyder_kernels/comms/commbase.py | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/spyder_kernels/comms/commbase.py b/spyder_kernels/comms/commbase.py index 27a6dfa9..f3374aa2 100644 --- a/spyder_kernels/comms/commbase.py +++ b/spyder_kernels/comms/commbase.py @@ -17,7 +17,6 @@ msg_dict = { 'spyder_msg_type': spyder_msg_type, 'content': content, - 'python_version': sys.version, } ``` @@ -95,7 +94,7 @@ def from_json(cls, json_data): etype, type(etype, (Exception,), {}) ) - instance.error = instance.etype(json_data["args"]) + instance.error = instance.etype(*json_data["args"]) instance.tb = traceback.StackSummary.from_list(json_data["tb"]) return instance @@ -248,7 +247,6 @@ def _send_message(self, spyder_msg_type, content=None, comm_id=None): msg_dict = { 'spyder_msg_type': spyder_msg_type, 'content': content, - 'python_version': sys.version, } self._comms[comm_id]['comm'].send(msg_dict) @@ -413,14 +411,14 @@ def _get_call_return_value(self, call_dict, comm_id): self._wait_reply(comm_id, call_id, call_name, timeout) - reply = self._reply_inbox.pop(call_id) + content = self._reply_inbox.pop(call_id) - value = reply['content']['call_return_value'] + return_value = content['call_return_value'] - if reply['is_error']: - return self._sync_error(value) + if content['is_error']: + return self._sync_error(return_value) - return value + return return_value def _wait_reply(self, comm_id, call_id, call_name, timeout): """ @@ -436,11 +434,12 @@ def _handle_remote_call_reply(self, msg_dict): call_id = content['call_id'] call_name = content['call_name'] is_error = content['is_error'] + return_value = content['call_return_value'] # Unexpected reply if call_id not in self._reply_waitlist: if is_error: - return self._async_error(msg_dict['call_return_value']) + return self._async_error(return_value) else: logger.debug('Got an unexpected reply {}, id:{}'.format( call_name, call_id)) @@ -450,18 +449,15 @@ def _handle_remote_call_reply(self, msg_dict): # Async error if is_error and not blocking: - return self._async_error(content['call_return_value']) + return self._async_error(return_value) # Callback if callback is not None and not is_error: - callback(content['call_return_value']) + callback(return_value) # Blocking inbox if blocking: - self._reply_inbox[call_id] = { - 'is_error': is_error, - 'content': content - } + self._reply_inbox[call_id] = content def _async_error(self, error_wrapper): """ From da975721a4c077c01669b460fd85b1984d1f7ab6 Mon Sep 17 00:00:00 2001 From: Quentin Peter Date: Sun, 26 May 2024 20:55:31 +0200 Subject: [PATCH 11/20] cloudpickle values --- spyder_kernels/console/kernel.py | 13 ++++++++++++- spyder_kernels/console/tests/test_console_kernel.py | 8 +++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/spyder_kernels/console/kernel.py b/spyder_kernels/console/kernel.py index 42714d6d..c7b5507d 100644 --- a/spyder_kernels/console/kernel.py +++ b/spyder_kernels/console/kernel.py @@ -11,6 +11,7 @@ """ # Standard library imports +import base64 import faulthandler import json import logging @@ -20,6 +21,7 @@ import traceback import tempfile import threading +import cloudpickle # Third-party imports from ipykernel.ipkernel import IPythonKernel @@ -359,11 +361,20 @@ def get_var_properties(self): def get_value(self, name): """Get the value of a variable""" ns = self.shell._get_current_namespace() - return ns[name] + value = ns[name] + + # Encode with cloudpickle and base64 + encoded_value = base64.b64encode( + cloudpickle.dumps(value) + ).decode() + + return encoded_value @comm_handler def set_value(self, name, value): """Set the value of a variable""" + # Decode_value + value = cloudpickle.loads(base64.b64decode(value.encode())) ns = self.shell._get_reference_namespace(name) ns[name] = value self.log.debug(ns) diff --git a/spyder_kernels/console/tests/test_console_kernel.py b/spyder_kernels/console/tests/test_console_kernel.py index 60f240d3..7819a5e1 100644 --- a/spyder_kernels/console/tests/test_console_kernel.py +++ b/spyder_kernels/console/tests/test_console_kernel.py @@ -22,6 +22,8 @@ import inspect import uuid from collections import namedtuple +import cloudpickle +import base64 # Test imports import pytest @@ -346,7 +348,11 @@ def test_set_value(kernel): name = 'a' asyncio.run(kernel.do_execute('a = 0', True)) value = 10 - kernel.set_value(name, value) + # Encode with cloudpickle and base64 + encoded_value = base64.b64encode( + cloudpickle.dumps(value) + ).decode() + kernel.set_value(name, encoded_value) log_text = get_log_text(kernel) assert "'__builtin__': Date: Sun, 26 May 2024 20:56:31 +0200 Subject: [PATCH 12/20] restore cloudpickle --- README.md | 1 + requirements/posix.txt | 1 + requirements/windows.txt | 1 + setup.py | 1 + 4 files changed, 4 insertions(+) diff --git a/README.md b/README.md index 686bfd7a..b6307c10 100644 --- a/README.md +++ b/README.md @@ -41,6 +41,7 @@ pip install spyder-kernels This project depends on: * [ipykernel](https://github.com/ipython/ipykernel/) +* [cloudpickle](https://github.com/cloudpipe/cloudpickle) * [wurlitzer](https://github.com/minrk/wurlitzer) (only on Linux and macOS). ## Changelog diff --git a/requirements/posix.txt b/requirements/posix.txt index cd73281e..0c22fa57 100644 --- a/requirements/posix.txt +++ b/requirements/posix.txt @@ -1,3 +1,4 @@ +cloudpickle ipykernel>=6.29.3,<7 ipython>=8.12.2,<9 jupyter_client>=7.4.9,<9 diff --git a/requirements/windows.txt b/requirements/windows.txt index 6289ad3e..9d6370b6 100644 --- a/requirements/windows.txt +++ b/requirements/windows.txt @@ -1,3 +1,4 @@ +cloudpickle ipykernel>=6.29.3,<7 ipython>=8.12.2,<9 jupyter_client>=7.4.9,<9 diff --git a/setup.py b/setup.py index 5e0b0f8f..79689b46 100644 --- a/setup.py +++ b/setup.py @@ -36,6 +36,7 @@ def get_version(module='spyder_kernels'): REQUIREMENTS = [ + 'cloudpickle', 'ipykernel>=6.29.3,<7', 'ipython>=8.12.2,<8.13; python_version=="3.8"', 'ipython>=8.13.0,<9,!=8.17.1; python_version>"3.8"', From 7a8ad427457033eaaf191a44ea80696ea6e55e58 Mon Sep 17 00:00:00 2001 From: Quentin Peter Date: Sun, 26 May 2024 21:27:47 +0200 Subject: [PATCH 13/20] encoded --- spyder_kernels/console/kernel.py | 21 ++++++++++--------- .../console/tests/test_console_kernel.py | 5 +---- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/spyder_kernels/console/kernel.py b/spyder_kernels/console/kernel.py index c7b5507d..1a90dffb 100644 --- a/spyder_kernels/console/kernel.py +++ b/spyder_kernels/console/kernel.py @@ -358,23 +358,24 @@ def get_var_properties(self): return None @comm_handler - def get_value(self, name): + def get_value(self, name, encoded=False): """Get the value of a variable""" ns = self.shell._get_current_namespace() value = ns[name] + if encoded: + # Encode with cloudpickle and base64 + value = base64.b64encode( + cloudpickle.dumps(value) + ).decode() - # Encode with cloudpickle and base64 - encoded_value = base64.b64encode( - cloudpickle.dumps(value) - ).decode() - - return encoded_value + return value @comm_handler - def set_value(self, name, value): + def set_value(self, name, value, encoded=False): """Set the value of a variable""" - # Decode_value - value = cloudpickle.loads(base64.b64decode(value.encode())) + if encoded: + # Decode_value + value = cloudpickle.loads(base64.b64decode(value.encode())) ns = self.shell._get_reference_namespace(name) ns[name] = value self.log.debug(ns) diff --git a/spyder_kernels/console/tests/test_console_kernel.py b/spyder_kernels/console/tests/test_console_kernel.py index 7819a5e1..b89d0b9b 100644 --- a/spyder_kernels/console/tests/test_console_kernel.py +++ b/spyder_kernels/console/tests/test_console_kernel.py @@ -349,10 +349,7 @@ def test_set_value(kernel): asyncio.run(kernel.do_execute('a = 0', True)) value = 10 # Encode with cloudpickle and base64 - encoded_value = base64.b64encode( - cloudpickle.dumps(value) - ).decode() - kernel.set_value(name, encoded_value) + kernel.set_value(name, value) log_text = get_log_text(kernel) assert "'__builtin__': Date: Mon, 27 May 2024 06:44:54 +0200 Subject: [PATCH 14/20] accept bytes in comms --- spyder_kernels/comms/commbase.py | 75 +++++++++++++++---- spyder_kernels/comms/frontendcomm.py | 2 +- spyder_kernels/console/kernel.py | 10 +-- .../console/tests/test_console_kernel.py | 3 - 4 files changed, 65 insertions(+), 25 deletions(-) diff --git a/spyder_kernels/comms/commbase.py b/spyder_kernels/comms/commbase.py index f3374aa2..cf7e7526 100644 --- a/spyder_kernels/comms/commbase.py +++ b/spyder_kernels/comms/commbase.py @@ -9,7 +9,7 @@ Comms transmit data in a list of buffers, and in a json-able dictionnary. Here, we only support json to avoid issues of compatibility between python -versions. +versions. In the abstraction below, buffers is used to send bytes. The messages exchanged have the following msg_dict: @@ -36,6 +36,8 @@ 'settings': A dictionnary of settings, 'call_args': The function args, 'call_kwargs': The function kwargs, + 'buffers_args_idx': The args index that are bytes, + 'buffers_kwargs_keys': the kwargs keys that are bytes } - If the 'settings' has `'blocking' = True`, a reply is sent. (spyder_msg_type = 'remote_call_reply'): @@ -225,7 +227,9 @@ def remote_call(self, comm_id=None, callback=None, **settings): return RemoteCallFactory(self, comm_id, callback, **settings) # ---- Private ----- - def _send_message(self, spyder_msg_type, content=None, comm_id=None): + def _send_message( + self, spyder_msg_type, content=None, comm_id=None, buffers=None + ): """ Publish custom messages to the other side. @@ -249,7 +253,7 @@ def _send_message(self, spyder_msg_type, content=None, comm_id=None): 'content': content, } - self._comms[comm_id]['comm'].send(msg_dict) + self._comms[comm_id]['comm'].send(msg_dict, buffers=buffers) @property def _comm_name(self): @@ -302,21 +306,34 @@ def _comm_message(self, msg): # Get message dict msg_dict = msg['content']['data'] spyder_msg_type = msg_dict['spyder_msg_type'] + buffers = msg['buffers'] if spyder_msg_type in self._message_handlers: - self._message_handlers[spyder_msg_type](msg_dict) + self._message_handlers[spyder_msg_type](msg_dict, buffers) else: logger.debug("No such spyder message type: %s" % spyder_msg_type) - def _handle_remote_call(self, msg): + def _handle_remote_call(self, msg, buffers): """Handle a remote call.""" msg_dict = msg['content'] self.on_incoming_call(msg_dict) try: + # read buffers + args = msg_dict['call_args'] + kwargs = msg_dict['call_kwargs'] + + if buffers: + for idx in msg_dict['buffers_args_idx']: + args[idx] = buffers.pop(0) + for name in msg_dict['buffers_kwargs_keys']: + kwargs[name] = buffers.pop(0) + assert len(buffers) == 0 + return_value = self._remote_callback( msg_dict['call_name'], - msg_dict['call_args'], - msg_dict['call_kwargs']) + args, + kwargs + ) self._set_call_return_value(msg_dict, return_value) except Exception: exc_infos = CommsErrorWrapper( @@ -350,6 +367,10 @@ def _set_call_return_value(self, call_dict, return_value, is_error=False): if not send_reply: # Nothing to send back return + buffers = None + if isinstance(return_value, bytes): + buffers = [return_value] + return_value = None content = { 'is_error': is_error, 'call_id': call_dict['call_id'], @@ -358,7 +379,8 @@ def _set_call_return_value(self, call_dict, return_value, is_error=False): } self._send_message( - 'remote_call_reply', content=content, comm_id=self.calling_comm_id + 'remote_call_reply', content=content, comm_id=self.calling_comm_id, + buffers=buffers ) def _register_call(self, call_dict, callback=None): @@ -379,11 +401,11 @@ def on_incoming_call(self, call_dict): """A call was received""" pass - def _send_call(self, call_dict, comm_id): + def _send_call(self, call_dict, comm_id, buffers=None): """Send call.""" call_dict = self.on_outgoing_call(call_dict) self._send_message( - 'remote_call', content=call_dict, comm_id=comm_id + 'remote_call', content=call_dict, comm_id=comm_id, buffers=buffers ) def _get_call_return_value(self, call_dict, comm_id): @@ -426,7 +448,7 @@ def _wait_reply(self, comm_id, call_id, call_name, timeout): """ raise NotImplementedError - def _handle_remote_call_reply(self, msg_dict): + def _handle_remote_call_reply(self, msg_dict, buffers): """ A blocking call received a reply. """ @@ -435,6 +457,12 @@ def _handle_remote_call_reply(self, msg_dict): call_name = content['call_name'] is_error = content['is_error'] return_value = content['call_return_value'] + if is_error: + return_value = CommsErrorWrapper.from_json(return_value) + elif buffers: + assert len(buffers) == 1 + return_value = buffers[0] + content['call_return_value'] = return_value # Unexpected reply if call_id not in self._reply_waitlist: @@ -463,13 +491,13 @@ def _async_error(self, error_wrapper): """ Handle an error that was raised on the other side asyncronously. """ - CommsErrorWrapper.from_json(error_wrapper).print_error() + error_wrapper.print_error() def _sync_error(self, error_wrapper): """ Handle an error that was raised on the other side syncronously. """ - CommsErrorWrapper.from_json(error_wrapper).raise_error() + error_wrapper.raise_error() class RemoteCallFactory: @@ -511,6 +539,23 @@ def __call__(self, *args, **kwargs): """ blocking = 'blocking' in self._settings and self._settings['blocking'] self._settings['send_reply'] = blocking or self._callback is not None + + # put all bytes in a buffer + buffers = [] + buffers_args_idx = [] + args = list(args) + for i, arg in enumerate(args): + if isinstance(arg, bytes): + buffers.append(arg) + buffers_args_idx.append(i) + args[i] = None + buffers_kwargs_keys = [] + for name in kwargs: + arg = kwargs[name] + if isinstance(arg, bytes): + buffers.append(arg) + buffers_kwargs_keys.append(name) + kwargs[name] = None call_id = uuid.uuid4().hex call_dict = { @@ -519,6 +564,8 @@ def __call__(self, *args, **kwargs): 'settings': self._settings, 'call_args': args, 'call_kwargs': kwargs, + 'buffers_args_idx': buffers_args_idx, + 'buffers_kwargs_keys': buffers_kwargs_keys } if not self._comms_wrapper.is_open(self._comm_id): @@ -528,6 +575,6 @@ def __call__(self, *args, **kwargs): logger.debug("Call to unconnected comm: %s" % self._name) return self._comms_wrapper._register_call(call_dict, self._callback) - self._comms_wrapper._send_call(call_dict, self._comm_id) + self._comms_wrapper._send_call(call_dict, self._comm_id, buffers) return self._comms_wrapper._get_call_return_value( call_dict, self._comm_id) diff --git a/spyder_kernels/comms/frontendcomm.py b/spyder_kernels/comms/frontendcomm.py index dd644b2e..514d5404 100644 --- a/spyder_kernels/comms/frontendcomm.py +++ b/spyder_kernels/comms/frontendcomm.py @@ -194,7 +194,7 @@ def _async_error(self, error_wrapper): """ Send an async error back to the frontend to be displayed. """ - self.remote_call()._async_error(error_wrapper) + self.remote_call()._async_error(error_wrapper.to_json()) def _register_comm(self, comm): """ diff --git a/spyder_kernels/console/kernel.py b/spyder_kernels/console/kernel.py index 1a90dffb..e29dd64e 100644 --- a/spyder_kernels/console/kernel.py +++ b/spyder_kernels/console/kernel.py @@ -11,7 +11,6 @@ """ # Standard library imports -import base64 import faulthandler import json import logging @@ -363,11 +362,8 @@ def get_value(self, name, encoded=False): ns = self.shell._get_current_namespace() value = ns[name] if encoded: - # Encode with cloudpickle and base64 - value = base64.b64encode( - cloudpickle.dumps(value) - ).decode() - + # Encode with cloudpickle + value = cloudpickle.dumps(value) return value @comm_handler @@ -375,7 +371,7 @@ def set_value(self, name, value, encoded=False): """Set the value of a variable""" if encoded: # Decode_value - value = cloudpickle.loads(base64.b64decode(value.encode())) + value = cloudpickle.loads(value) ns = self.shell._get_reference_namespace(name) ns[name] = value self.log.debug(ns) diff --git a/spyder_kernels/console/tests/test_console_kernel.py b/spyder_kernels/console/tests/test_console_kernel.py index b89d0b9b..60f240d3 100644 --- a/spyder_kernels/console/tests/test_console_kernel.py +++ b/spyder_kernels/console/tests/test_console_kernel.py @@ -22,8 +22,6 @@ import inspect import uuid from collections import namedtuple -import cloudpickle -import base64 # Test imports import pytest @@ -348,7 +346,6 @@ def test_set_value(kernel): name = 'a' asyncio.run(kernel.do_execute('a = 0', True)) value = 10 - # Encode with cloudpickle and base64 kernel.set_value(name, value) log_text = get_log_text(kernel) assert "'__builtin__': Date: Mon, 27 May 2024 08:06:14 +0200 Subject: [PATCH 15/20] save return value --- spyder_kernels/comms/commbase.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spyder_kernels/comms/commbase.py b/spyder_kernels/comms/commbase.py index cf7e7526..260454bb 100644 --- a/spyder_kernels/comms/commbase.py +++ b/spyder_kernels/comms/commbase.py @@ -457,12 +457,13 @@ def _handle_remote_call_reply(self, msg_dict, buffers): call_name = content['call_name'] is_error = content['is_error'] return_value = content['call_return_value'] + # Prepare return value if is_error: return_value = CommsErrorWrapper.from_json(return_value) elif buffers: assert len(buffers) == 1 return_value = buffers[0] - content['call_return_value'] = return_value + content['call_return_value'] = return_value # Unexpected reply if call_id not in self._reply_waitlist: From 14ce15c2b495b1f0be68fa4c24161fcdb4524a66 Mon Sep 17 00:00:00 2001 From: Quentin Peter Date: Mon, 27 May 2024 22:01:18 +0200 Subject: [PATCH 16/20] update comments --- spyder_kernels/comms/commbase.py | 48 +++++++++++++++++--------------- 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/spyder_kernels/comms/commbase.py b/spyder_kernels/comms/commbase.py index 260454bb..612a8ea1 100644 --- a/spyder_kernels/comms/commbase.py +++ b/spyder_kernels/comms/commbase.py @@ -36,18 +36,20 @@ 'settings': A dictionnary of settings, 'call_args': The function args, 'call_kwargs': The function kwargs, - 'buffers_args_idx': The args index that are bytes, - 'buffers_kwargs_keys': the kwargs keys that are bytes - } + 'buffered_args': The args index that are in the buffers, + 'buffered_kwargs': the kwargs keys that are in the buffers + } + - The buffer contains any bytes in the arguments - If the 'settings' has `'blocking' = True`, a reply is sent. (spyder_msg_type = 'remote_call_reply'): - The 'content' is a dict with: { - 'is_error': a boolean indicating if the return value is an - exception to be raised. - 'call_id': The uuid from above, - 'call_name': The function name (mostly for debugging), - 'call_return_value': The return value of the function - } + 'is_error': a boolean indicating if the return value is an + exception to be raised. + 'call_id': The uuid from above, + 'call_name': The function name (mostly for debugging), + 'call_return_value': The return value of the function + } + - The buffer contains the return value if it is bytes """ import logging import sys @@ -323,17 +325,17 @@ def _handle_remote_call(self, msg, buffers): kwargs = msg_dict['call_kwargs'] if buffers: - for idx in msg_dict['buffers_args_idx']: + for idx in msg_dict['buffered_args']: args[idx] = buffers.pop(0) - for name in msg_dict['buffers_kwargs_keys']: + for name in msg_dict['buffered_kwargs']: kwargs[name] = buffers.pop(0) assert len(buffers) == 0 return_value = self._remote_callback( - msg_dict['call_name'], - args, - kwargs - ) + msg_dict['call_name'], + args, + kwargs + ) self._set_call_return_value(msg_dict, return_value) except Exception: exc_infos = CommsErrorWrapper( @@ -536,26 +538,26 @@ def __call__(self, *args, **kwargs): """ Transmit the call to the other side of the tunnel. - The args and kwargs have to be picklable. + The args and kwargs have to be JSON-serializable or bytes. """ blocking = 'blocking' in self._settings and self._settings['blocking'] self._settings['send_reply'] = blocking or self._callback is not None - # put all bytes in a buffer + # The call will be serialized with json. The bytes are sent separately. buffers = [] - buffers_args_idx = [] + buffered_args = [] + buffered_kwargs = [] args = list(args) for i, arg in enumerate(args): if isinstance(arg, bytes): buffers.append(arg) - buffers_args_idx.append(i) + buffered_args.append(i) args[i] = None - buffers_kwargs_keys = [] for name in kwargs: arg = kwargs[name] if isinstance(arg, bytes): buffers.append(arg) - buffers_kwargs_keys.append(name) + buffered_kwargs.append(name) kwargs[name] = None call_id = uuid.uuid4().hex @@ -565,8 +567,8 @@ def __call__(self, *args, **kwargs): 'settings': self._settings, 'call_args': args, 'call_kwargs': kwargs, - 'buffers_args_idx': buffers_args_idx, - 'buffers_kwargs_keys': buffers_kwargs_keys + 'buffered_args': buffered_args, + 'buffered_kwargs': buffered_kwargs } if not self._comms_wrapper.is_open(self._comm_id): From 017de4706a65a7ffe9b4d28483933264a693298b Mon Sep 17 00:00:00 2001 From: Quentin Peter Date: Mon, 27 May 2024 22:12:35 +0200 Subject: [PATCH 17/20] update comments --- spyder_kernels/comms/commbase.py | 2 -- spyder_kernels/console/kernel.py | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/spyder_kernels/comms/commbase.py b/spyder_kernels/comms/commbase.py index 612a8ea1..2cfb8d12 100644 --- a/spyder_kernels/comms/commbase.py +++ b/spyder_kernels/comms/commbase.py @@ -420,7 +420,6 @@ def _get_call_return_value(self, call_dict, comm_id): settings = call_dict['settings'] blocking = 'blocking' in settings and settings['blocking'] - if not blocking: return @@ -441,7 +440,6 @@ def _get_call_return_value(self, call_dict, comm_id): if content['is_error']: return self._sync_error(return_value) - return return_value def _wait_reply(self, comm_id, call_id, call_name, timeout): diff --git a/spyder_kernels/console/kernel.py b/spyder_kernels/console/kernel.py index e29dd64e..449df0e7 100644 --- a/spyder_kernels/console/kernel.py +++ b/spyder_kernels/console/kernel.py @@ -275,7 +275,7 @@ def get_current_frames(self, ignore_internal_threads=True): thread_name = thread_names[thread_id] else: thread_name = str(thread_id) - # Transform stack in a named tuple because FrameSummary objects + # Transform stack in a dict because FrameSummary objects # are not compatible between versions of python frames[thread_name] = [ { From 94d107e4fecf4a1b910feb41b96e6f228ab3f9c1 Mon Sep 17 00:00:00 2001 From: Quentin Peter Date: Tue, 28 May 2024 07:13:56 +0200 Subject: [PATCH 18/20] stacksummary_to_json --- spyder_kernels/comms/commbase.py | 39 +++++++++++++++++++++------ spyder_kernels/console/kernel.py | 17 ++++-------- spyder_kernels/console/shell.py | 12 ++------- spyder_kernels/customize/spyderpdb.py | 18 +++---------- 4 files changed, 42 insertions(+), 44 deletions(-) diff --git a/spyder_kernels/comms/commbase.py b/spyder_kernels/comms/commbase.py index 2cfb8d12..8d86cf5f 100644 --- a/spyder_kernels/comms/commbase.py +++ b/spyder_kernels/comms/commbase.py @@ -68,6 +68,32 @@ class CommError(RuntimeError): pass +def stacksummary_to_json(stack): + """StackSummary to json.""" + return [ + { + "filename": frame.filename, + "lineno": frame.lineno, + "name": frame.name, + "line": frame.line + } + for frame in stack + ] + + +def staksummary_from_json(stack): + """StackSummary from json.""" + traceback.StackSummary.from_list([ + ( + frame["filename"], + frame["lineno"], + frame["name"], + frame["line"] + ) + for frame in stack + ]) + + class CommsErrorWrapper(): def __init__(self, call_name, call_id): self.call_name = call_name @@ -81,10 +107,7 @@ def to_json(self): "call_id": self.call_id, "etype": self.etype.__name__, "args": self.error.args, - "tb": [ - (frame.filename, frame.lineno, frame.name, frame.line) - for frame in self.tb - ] + "tb": stacksummary_to_json(self.tb) } @classmethod @@ -99,7 +122,7 @@ def from_json(cls, json_data): type(etype, (Exception,), {}) ) instance.error = instance.etype(*json_data["args"]) - instance.tb = traceback.StackSummary.from_list(json_data["tb"]) + instance.tb = staksummary_from_json(json_data["tb"]) return instance def raise_error(self): @@ -323,14 +346,14 @@ def _handle_remote_call(self, msg, buffers): # read buffers args = msg_dict['call_args'] kwargs = msg_dict['call_kwargs'] - + if buffers: for idx in msg_dict['buffered_args']: args[idx] = buffers.pop(0) for name in msg_dict['buffered_kwargs']: kwargs[name] = buffers.pop(0) assert len(buffers) == 0 - + return_value = self._remote_callback( msg_dict['call_name'], args, @@ -540,7 +563,7 @@ def __call__(self, *args, **kwargs): """ blocking = 'blocking' in self._settings and self._settings['blocking'] self._settings['send_reply'] = blocking or self._callback is not None - + # The call will be serialized with json. The bytes are sent separately. buffers = [] buffered_args = [] diff --git a/spyder_kernels/console/kernel.py b/spyder_kernels/console/kernel.py index 449df0e7..3a6bfa22 100644 --- a/spyder_kernels/console/kernel.py +++ b/spyder_kernels/console/kernel.py @@ -31,6 +31,7 @@ # Local imports import spyder_kernels +from spyder_kernels.comms.commbase import stacksummary_to_json from spyder_kernels.comms.frontendcomm import FrontendComm from spyder_kernels.comms.decorators import ( register_comm_handlers, comm_handler) @@ -258,7 +259,7 @@ def get_current_frames(self, ignore_internal_threads=True): """Get the current frames.""" ignore_list = self.get_system_threads_id() main_id = threading.main_thread().ident - frames = {} + stack_dict = {} thread_names = {thread.ident: thread.name for thread in threading.enumerate()} @@ -277,16 +278,8 @@ def get_current_frames(self, ignore_internal_threads=True): thread_name = str(thread_id) # Transform stack in a dict because FrameSummary objects # are not compatible between versions of python - frames[thread_name] = [ - { - "filename": frame.filename, - "lineno": frame.lineno, - "name": frame.name, - "line": frame.line - } - for frame in stack - ] - return frames + stack_dict[thread_name] = stacksummary_to_json(stack) + return stack_dict # --- For the Variable Explorer @comm_handler @@ -362,7 +355,7 @@ def get_value(self, name, encoded=False): ns = self.shell._get_current_namespace() value = ns[name] if encoded: - # Encode with cloudpickle + # Encode with cloudpickle value = cloudpickle.dumps(value) return value diff --git a/spyder_kernels/console/shell.py b/spyder_kernels/console/shell.py index 88a1162e..5a9c1a29 100644 --- a/spyder_kernels/console/shell.py +++ b/spyder_kernels/console/shell.py @@ -26,6 +26,7 @@ from spyder_kernels.customize.namespace_manager import NamespaceManager from spyder_kernels.customize.spyderpdb import SpyderPdb from spyder_kernels.customize.code_runner import SpyderCodeRunner +from spyder_kernels.comms.commbase import stacksummary_to_json from spyder_kernels.comms.decorators import comm_handler from spyder_kernels.utils.mpl import automatic_backend @@ -273,16 +274,7 @@ def showtraceback(self, exc_tuple=None, filename=None, tb_offset=None, etype, value, tb = self._get_exc_info(exc_tuple) etype = etype.__name__ value = value.args - stack = traceback.extract_tb(tb.tb_next) - stack = [ - { - "filename": frame.filename, - "lineno": frame.lineno, - "name": frame.name, - "line": frame.line - } - for frame in stack - ] + stack = stacksummary_to_json(traceback.extract_tb(tb.tb_next)) self.kernel.frontend_call(blocking=False).show_traceback( etype, value, stack) except Exception: diff --git a/spyder_kernels/customize/spyderpdb.py b/spyder_kernels/customize/spyderpdb.py index 4bdee99a..a522afa5 100755 --- a/spyder_kernels/customize/spyderpdb.py +++ b/spyder_kernels/customize/spyderpdb.py @@ -24,6 +24,7 @@ from IPython.core.inputtransformer2 import TransformerManager import spyder_kernels +from spyder_kernels.comms.commbase import stacksummary_to_json from spyder_kernels.comms.frontendcomm import CommError, frontend_request from spyder_kernels.customize.utils import ( path_is_library, capture_last_Expr, exec_encapsulate_locals @@ -829,21 +830,10 @@ def get_pdb_state(self): if self.pdb_publish_stack: # Publish Pdb stack so we can update the Debugger plugin on Spyder - pdb_stack = traceback.StackSummary.extract(self.stack) - - pdb_stack = [ - { - "filename": frame.filename, - "lineno": frame.lineno, - "name": frame.name, - "line": frame.line - } - for frame in pdb_stack - ] - - + pdb_stack = stacksummary_to_json( + traceback.StackSummary.extract(self.stack) + ) pdb_index = self.curindex - skip_hidden = getattr(self, 'skip_hidden', False) if skip_hidden: From 4f3814aef26847b1cb94d83d5396b08f9f7a885a Mon Sep 17 00:00:00 2001 From: Quentin Peter Date: Tue, 28 May 2024 08:11:21 +0200 Subject: [PATCH 19/20] pep8 and comments --- spyder_kernels/comms/commbase.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/spyder_kernels/comms/commbase.py b/spyder_kernels/comms/commbase.py index 8d86cf5f..ddc90ee9 100644 --- a/spyder_kernels/comms/commbase.py +++ b/spyder_kernels/comms/commbase.py @@ -102,6 +102,7 @@ def __init__(self, call_name, call_id): self.tb = traceback.extract_tb(tb) def to_json(self): + """Create JSON representation.""" return { "call_name": self.call_name, "call_id": self.call_id, @@ -112,6 +113,7 @@ def to_json(self): @classmethod def from_json(cls, json_data): + """Get a CommsErrorWrapper from a JSON representation.""" instance = cls.__new__(cls) instance.call_name = json_data["call_name"] instance.call_id = json_data["call_id"] @@ -253,8 +255,8 @@ def remote_call(self, comm_id=None, callback=None, **settings): # ---- Private ----- def _send_message( - self, spyder_msg_type, content=None, comm_id=None, buffers=None - ): + self, spyder_msg_type, content=None, comm_id=None, buffers=None + ): """ Publish custom messages to the other side. @@ -264,10 +266,11 @@ def _send_message( The spyder message type content: dict The (JSONable) content of the message - data: any - Any object that is serializable by json. comm_id: int the comm to send to. If None sends to all comms. + buffers: list(bytes) + a list of bytes to send + """ if not self.is_open(comm_id): raise CommError("The comm is not connected.") From ab4ea19f1d1425a920a374fcbcf644621c28dcce Mon Sep 17 00:00:00 2001 From: Quentin Peter Date: Tue, 28 May 2024 22:52:05 +0200 Subject: [PATCH 20/20] Apply suggestions from code review Co-authored-by: Carlos Cordoba --- spyder_kernels/comms/commbase.py | 23 ++++++++++++++--------- spyder_kernels/console/kernel.py | 5 ++++- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/spyder_kernels/comms/commbase.py b/spyder_kernels/comms/commbase.py index ddc90ee9..800078f9 100644 --- a/spyder_kernels/comms/commbase.py +++ b/spyder_kernels/comms/commbase.py @@ -8,7 +8,7 @@ Class that handles communications between Spyder kernel and frontend. Comms transmit data in a list of buffers, and in a json-able dictionnary. -Here, we only support json to avoid issues of compatibility between python +Here, we only support json to avoid issues of compatibility between Python versions. In the abstraction below, buffers is used to send bytes. The messages exchanged have the following msg_dict: @@ -17,7 +17,7 @@ msg_dict = { 'spyder_msg_type': spyder_msg_type, 'content': content, - } + } ``` To simplify the usage of messaging, we use a higher level function calling @@ -269,8 +269,7 @@ def _send_message( comm_id: int the comm to send to. If None sends to all comms. buffers: list(bytes) - a list of bytes to send - + a list of bytes to send. """ if not self.is_open(comm_id): raise CommError("The comm is not connected.") @@ -279,7 +278,7 @@ def _send_message( msg_dict = { 'spyder_msg_type': spyder_msg_type, 'content': content, - } + } self._comms[comm_id]['comm'].send(msg_dict, buffers=buffers) @@ -318,7 +317,7 @@ def _register_comm(self, comm): self._comms[comm.comm_id] = { 'comm': comm, 'status': 'opening', - } + } def _comm_close(self, msg): """Close comm.""" @@ -395,10 +394,12 @@ def _set_call_return_value(self, call_dict, return_value, is_error=False): if not send_reply: # Nothing to send back return + buffers = None if isinstance(return_value, bytes): buffers = [return_value] return_value = None + content = { 'is_error': is_error, 'call_id': call_dict['call_id'], @@ -407,7 +408,9 @@ def _set_call_return_value(self, call_dict, return_value, is_error=False): } self._send_message( - 'remote_call_reply', content=content, comm_id=self.calling_comm_id, + 'remote_call_reply', + content=content, + comm_id=self.calling_comm_id, buffers=buffers ) @@ -461,7 +464,6 @@ def _get_call_return_value(self, call_dict, comm_id): self._wait_reply(comm_id, call_id, call_name, timeout) content = self._reply_inbox.pop(call_id) - return_value = content['call_return_value'] if content['is_error']: @@ -483,6 +485,7 @@ def _handle_remote_call_reply(self, msg_dict, buffers): call_name = content['call_name'] is_error = content['is_error'] return_value = content['call_return_value'] + # Prepare return value if is_error: return_value = CommsErrorWrapper.from_json(return_value) @@ -572,11 +575,13 @@ def __call__(self, *args, **kwargs): buffered_args = [] buffered_kwargs = [] args = list(args) + for i, arg in enumerate(args): if isinstance(arg, bytes): buffers.append(arg) buffered_args.append(i) args[i] = None + for name in kwargs: arg = kwargs[name] if isinstance(arg, bytes): @@ -593,7 +598,7 @@ def __call__(self, *args, **kwargs): 'call_kwargs': kwargs, 'buffered_args': buffered_args, 'buffered_kwargs': buffered_kwargs - } + } if not self._comms_wrapper.is_open(self._comm_id): # Only an error if the call is blocking. diff --git a/spyder_kernels/console/kernel.py b/spyder_kernels/console/kernel.py index 3a6bfa22..da595371 100644 --- a/spyder_kernels/console/kernel.py +++ b/spyder_kernels/console/kernel.py @@ -276,9 +276,11 @@ def get_current_frames(self, ignore_internal_threads=True): thread_name = thread_names[thread_id] else: thread_name = str(thread_id) + # Transform stack in a dict because FrameSummary objects - # are not compatible between versions of python + # are not compatible between versions of Python stack_dict[thread_name] = stacksummary_to_json(stack) + return stack_dict # --- For the Variable Explorer @@ -365,6 +367,7 @@ def set_value(self, name, value, encoded=False): if encoded: # Decode_value value = cloudpickle.loads(value) + ns = self.shell._get_reference_namespace(name) ns[name] = value self.log.debug(ns)