Skip to content

Commit

Permalink
Use json instead of cloudpickle
Browse files Browse the repository at this point in the history
  • Loading branch information
Quentin Peter committed May 25, 2024
1 parent 3448ea1 commit 8edb4c2
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 62 deletions.
76 changes: 35 additions & 41 deletions spyder_kernels/comms/commbase.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Expand Down Expand Up @@ -50,8 +50,6 @@
'call_name': The function name (mostly for debugging)
}
"""
import cloudpickle
import pickle
import logging
import sys
import uuid
Expand All @@ -60,8 +58,6 @@

logger = logging.getLogger(__name__)

DEFAULT_PICKLE_PROTOCOL = 4

# Max timeout (in secs) for blocking calls
TIMEOUT = 3

Expand All @@ -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):
"""
Expand Down Expand Up @@ -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.
"""
Expand All @@ -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):
Expand Down Expand Up @@ -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',
}

Expand All @@ -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:
Expand All @@ -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'],
Expand Down Expand Up @@ -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:
Expand All @@ -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."""
Expand Down Expand Up @@ -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:
Expand Down
2 changes: 0 additions & 2 deletions spyder_kernels/comms/frontendcomm.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
20 changes: 6 additions & 14 deletions spyder_kernels/console/kernel.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""

Expand Down Expand Up @@ -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
Expand Down
6 changes: 2 additions & 4 deletions spyder_kernels/console/tests/test_console_kernel.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down
1 change: 0 additions & 1 deletion spyder_kernels/customize/namespace_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions spyder_kernels/customize/spyderpdb.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 8edb4c2

Please sign in to comment.