Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PR: Refactor the way clients are created (IPython console) #19062

Merged
merged 35 commits into from
Sep 19, 2022
Merged
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
b21b0ca
refactor kernel connection
Aug 24, 2022
f208b16
remove comm less eagerly
Aug 25, 2022
c8b6be6
Merge branch 'spyder_kernels_version' into tmp
Aug 25, 2022
8c56302
put back close comm
Aug 25, 2022
dbc7b38
Merge remote-tracking branch 'upstream/master' into refactor_createCl…
Aug 25, 2022
e2d2a8f
move restart message
Aug 25, 2022
232827e
remove after shutdown
Aug 25, 2022
e1c49eb
PYTEST_CURRENT_TEST
Aug 25, 2022
708c797
only closing
Aug 25, 2022
58dcb03
replace std files
Aug 26, 2022
d73660f
Apply suggestions from code review
impact27 Aug 29, 2022
ba03b92
fix
Aug 29, 2022
3da1619
kernel handler
Aug 30, 2022
2b9a68d
fix import
Aug 30, 2022
295811f
fix import
Aug 30, 2022
4087e34
Merge branch 'master' into refactor_createClientWidget
Sep 2, 2022
8f19ede
Merge branch 'master' into refactor_createClientWidget
Sep 3, 2022
307a877
move send spyder kernel configuration
Sep 3, 2022
d26b5fb
move fault
Sep 3, 2022
91d6546
Merge remote-tracking branch 'upstream/master' into refactor_createCl…
Sep 6, 2022
ffbf471
Merge remote-tracking branch 'upstream/master' into refactor_createCl…
Sep 12, 2022
c0b45b4
remove unused import
Sep 13, 2022
cf4ace9
fix error on start
Sep 13, 2022
a6b496b
fix text
Sep 13, 2022
50e2da9
only setup after checking version
Sep 13, 2022
88e59c8
remove double call to config
Sep 13, 2022
f4cecc8
improve test
Sep 13, 2022
d573177
do not interrupt on setup
Sep 16, 2022
6bf2300
remove _read_stderr
Sep 16, 2022
62caab8
notify_deleted
Sep 16, 2022
3ffc521
pep8
Sep 17, 2022
eb157ee
Merge remote-tracking branch 'upstream/master' into refactor_createCl…
Sep 18, 2022
66b9a15
Apply suggestions from code review
impact27 Sep 19, 2022
2db91a1
Apply suggestions from code review
impact27 Sep 19, 2022
0fee0f4
move CachedKernelMixin
Sep 19, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 0 additions & 17 deletions spyder/api/shellconnect/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ def on_ipython_console_available(self):
ipyconsole.sig_shellwidget_changed.connect(self.set_shellwidget)
ipyconsole.sig_shellwidget_created.connect(self.add_shellwidget)
ipyconsole.sig_shellwidget_deleted.connect(self.remove_shellwidget)
ipyconsole.sig_external_spyder_kernel_connected.connect(
ccordoba12 marked this conversation as resolved.
Show resolved Hide resolved
self.on_connection_to_external_spyder_kernel)

@on_plugin_teardown(plugin=Plugins.IPythonConsole)
def on_ipython_console_teardown(self):
Expand All @@ -43,8 +41,6 @@ def on_ipython_console_teardown(self):
ipyconsole.sig_shellwidget_changed.disconnect(self.set_shellwidget)
ipyconsole.sig_shellwidget_created.disconnect(self.add_shellwidget)
ipyconsole.sig_shellwidget_deleted.disconnect(self.remove_shellwidget)
ipyconsole.sig_external_spyder_kernel_connected.disconnect(
self.on_connection_to_external_spyder_kernel)

# ---- Public API
# -------------------------------------------------------------------------
Expand Down Expand Up @@ -110,16 +106,3 @@ def get_widget_for_shellwidget(self, shellwidget):
The widget corresponding to the shellwidget, or None if not found.
"""
return self.get_widget().get_widget_for_shellwidget(shellwidget)

def on_connection_to_external_spyder_kernel(self, shellwidget):
"""
Actions to take when the IPython console connects to an
external Spyder kernel.

Parameters
----------
shellwidget: spyder.plugins.ipyconsole.widgets.shell.ShellWidget
The shell widget that was connected to the external Spyder
kernel.
"""
pass
12 changes: 9 additions & 3 deletions spyder/app/tests/test_mainwindow.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
DebuggerWidgetActions, DebuggerToolbarActions)
from spyder.plugins.help.widgets import ObjectComboBox
from spyder.plugins.help.tests.test_plugin import check_text
from spyder.plugins.ipythonconsole.utils.kernel_handler import KernelHandler
from spyder.plugins.layout.layouts import DefaultLayouts
from spyder.plugins.toolbar.api import ApplicationToolbars
from spyder.py3compat import PY2, qbytearray_to_str, to_text_string
Expand Down Expand Up @@ -120,6 +121,12 @@ def test_leaks(main_window, qtbot):
Many other ways of leaking exist but are not covered here.
"""
def wait_all_shutdown():
objects = gc.get_objects()
for o in objects:
if isinstance(o, KernelHandler):
o.wait_shutdown_thread()

def ns_fun(main_window, qtbot):
# Wait until the window is fully up
shell = main_window.ipyconsole.get_current_shellwidget()
Expand All @@ -129,7 +136,7 @@ def ns_fun(main_window, qtbot):
# Count initial objects
# Only one of each should be present, but because of many leaks,
# this is most likely not the case. Here only closing is tested
shell.wait_all_shutdown()
wait_all_shutdown()
gc.collect()
objects = gc.get_objects()
n_code_editor_init = 0
Expand Down Expand Up @@ -160,8 +167,7 @@ def ns_fun(main_window, qtbot):
main_window.ipyconsole.restart()

# Wait until the shells are closed
shell = main_window.ipyconsole.get_current_shellwidget()
shell.wait_all_shutdown()
wait_all_shutdown()
return n_shell_init, n_code_editor_init

n_shell_init, n_code_editor_init = ns_fun(main_window, qtbot)
Expand Down
14 changes: 13 additions & 1 deletion spyder/plugins/ipythonconsole/comms/kernelcomm.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,16 @@ def __init__(self):
# Register handlers
self.register_call_handler('_async_error', self._async_error)

def is_open(self, comm_id=None):
"""Check to see if the comm is open."""
valid_comms = [
comm for comm in self._comms
if self._comms[comm]['status'] in ['opening', 'ready']
]
if comm_id is None:
return len(valid_comms) > 0
return comm_id in valid_comms

@contextmanager
def comm_channel_manager(self, comm_id, queue_message=False):
"""Use control_channel instead of shell_channel."""
Expand All @@ -63,14 +73,16 @@ def _set_call_return_value(self, call_dict, data, is_error=False):
super(KernelComm, self)._set_call_return_value(
call_dict, data, is_error)

def remove(self, comm_id=None):
def remove(self, comm_id=None, only_closing=False):
"""
Remove the comm without notifying the other side.
Use when the other side is already down.
"""
id_list = self.get_comm_id_list(comm_id)
for comm_id in id_list:
if only_closing and self._comms[comm_id]['status'] != 'closing':
continue
del self._comms[comm_id]

def close(self, comm_id=None):
Expand Down
12 changes: 0 additions & 12 deletions spyder/plugins/ipythonconsole/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,16 +138,6 @@ class IPythonConsole(SpyderDockablePlugin):
The shellwigdet.
"""

sig_external_spyder_kernel_connected = Signal(object)
"""
This signal is emitted when we connect to an external Spyder kernel.
Parameters
----------
shellwidget: spyder.plugins.ipyconsole.widgets.shell.ShellWidget
The shellwigdet that was connected to the kernel.
"""

sig_render_plain_text_requested = Signal(str)
"""
This signal is emitted to request a plain text help render.
Expand Down Expand Up @@ -216,8 +206,6 @@ def on_initialize(self):
widget.sig_shellwidget_created.connect(self.sig_shellwidget_created)
widget.sig_shellwidget_deleted.connect(self.sig_shellwidget_deleted)
widget.sig_shellwidget_changed.connect(self.sig_shellwidget_changed)
widget.sig_external_spyder_kernel_connected.connect(
self.sig_external_spyder_kernel_connected)
widget.sig_render_plain_text_requested.connect(
self.sig_render_plain_text_requested)
widget.sig_render_rich_text_requested.connect(
Expand Down
38 changes: 23 additions & 15 deletions spyder/plugins/ipythonconsole/tests/test_ipythonconsole.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
from spyder.plugins.help.tests.test_plugin import check_text
from spyder.plugins.help.utils.sphinxify import CSS_PATH
from spyder.plugins.ipythonconsole.plugin import IPythonConsole
from spyder.plugins.ipythonconsole.utils import stdfile
from spyder.plugins.ipythonconsole.utils.style import create_style_class
from spyder.plugins.ipythonconsole.widgets import ClientWidget
from spyder.utils.programs import get_temp_dir
Expand Down Expand Up @@ -159,9 +160,9 @@ def __getattr__(self, attr):
# Instruct the console to not use a stderr file
no_stderr_file = request.node.get_closest_marker('no_stderr_file')
if no_stderr_file:
test_no_stderr = 'True'
test_no_stderr = True
else:
test_no_stderr = ''
test_no_stderr = False

# Use the automatic backend if requested
auto_backend = request.node.get_closest_marker('auto_backend')
Expand Down Expand Up @@ -211,8 +212,8 @@ def __getattr__(self, attr):

# Create the console and a new client and set environment
os.environ['IPYCONSOLE_TESTING'] = 'True'
os.environ['IPYCONSOLE_TEST_DIR'] = test_dir
os.environ['IPYCONSOLE_TEST_NO_STDERR'] = test_no_stderr
stdfile.IPYCONSOLE_TEST_DIR = test_dir
stdfile.IPYCONSOLE_TEST_NO_STDERR = test_no_stderr
window = MainWindowMock()
console = IPythonConsole(parent=window, configuration=configuration)

Expand Down Expand Up @@ -288,8 +289,8 @@ def get_plugin(name):
# Close
console.on_close()
os.environ.pop('IPYCONSOLE_TESTING')
os.environ.pop('IPYCONSOLE_TEST_DIR')
os.environ.pop('IPYCONSOLE_TEST_NO_STDERR')
stdfile.IPYCONSOLE_TEST_DIR = None
stdfile.IPYCONSOLE_TEST_NO_STDERR = False

if os.name == 'nt' or known_leak:
# Do not test for leaks
Expand Down Expand Up @@ -1306,7 +1307,8 @@ def test_set_elapsed_time(ipyconsole, qtbot):
# Set time to 2 minutes ago.
client.t0 -= 120
with qtbot.waitSignal(client.timer.timeout, timeout=5000):
ipyconsole.get_widget().set_client_elapsed_time(client)
client.timer.timeout.connect(client.show_time)
client.timer.start(1000)
assert ('00:02:00' in main_widget.time_label.text() or
'00:02:01' in main_widget.time_label.text())

Expand Down Expand Up @@ -1401,10 +1403,9 @@ def test_kernel_crash(ipyconsole, qtbot):
ipyconsole.create_new_client()

# Assert that the console is showing an error
qtbot.waitUntil(lambda: ipyconsole.get_clients()[-1].is_error_shown,
timeout=6000)
error_client = ipyconsole.get_clients()[-1]
assert error_client.is_error_shown
qtbot.waitUntil(lambda: bool(error_client.error_text), timeout=6000)
assert error_client.error_text

# Assert the error contains the text we expect
webview = error_client.infowidget
Expand Down Expand Up @@ -1636,10 +1637,17 @@ def test_pdb_ignore_lib(ipyconsole, qtbot, show_lib):
control.setFocus()

# Tests assume inline backend
qtbot.wait(1000)
ipyconsole.set_conf('pdb_ignore_lib', not show_lib, section="debugger")
qtbot.wait(1000)
with qtbot.waitSignal(shell.executed):
shell.execute('%debug print()')

with qtbot.waitSignal(shell.executed):
shell.execute(
'"value = " + str(get_ipython().pdb_session.pdb_ignore_lib)')
assert "value = " + str(not show_lib) in control.toPlainText()

qtbot.keyClicks(control, '!s')
with qtbot.waitSignal(shell.executed):
qtbot.keyClick(control, Qt.Key_Enter)
Expand Down Expand Up @@ -2412,14 +2420,12 @@ def test_old_kernel_version(ipyconsole, qtbot):
"""
# Set a false _spyder_kernels_version in the cached kernel
w = ipyconsole.get_widget()
# create new client so PYTEST_CURRENT_TEST is the same
w.create_new_client()
# Wait until the window is fully up
shell = ipyconsole.get_current_shellwidget()
qtbot.waitUntil(
lambda: shell._prompt_html is not None, timeout=SHELL_TIMEOUT)

kc = w._cached_kernel_properties[-1][2]
kc = w._cached_kernel_properties[-1].kernel_client
kc.start_channels()
kc.execute("get_ipython()._spyder_kernels_version = ('1.0.0', '')")
# Cleanup the kernel_client so it can be used again
Expand All @@ -2435,8 +2441,10 @@ def test_old_kernel_version(ipyconsole, qtbot):
client = w.get_current_client()

# Make sure an error is shown
qtbot.waitUntil(lambda: client.error_text is not None)
assert '1.0.0' in client.error_text
control = client.get_control()
qtbot.waitUntil(
lambda: "1.0.0" in control.toPlainText(), timeout=SHELL_TIMEOUT)
assert "conda install spyder" in control.toPlainText()


def test_run_script(ipyconsole, qtbot, tmp_path):
Expand Down
Loading