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

Fix comms and add qtconsole downstream test #1056

Merged
merged 14 commits into from Dec 19, 2022
36 changes: 36 additions & 0 deletions .github/workflows/downstream.yml
Expand Up @@ -84,6 +84,41 @@ jobs:
pip install -e ".[test]"
python test_ipykernel.py

qtconsole:
runs-on: ubuntu-latest
timeout-minutes: 20
steps:
- name: Checkout
uses: actions/checkout@v3

- name: Setup Python
uses: actions/setup-python@v4
with:
python-version: "3.9"
architecture: "x64"

- name: Install System Packages
run: |
sudo apt-get update
sudo apt-get install -y --no-install-recommends '^libxcb.*-dev' libx11-xcb-dev libglu1-mesa-dev libxrender-dev libxi-dev libxkbcommon-dev libxkbcommon-x11-dev
- name: Install qtconsole dependencies
shell: bash -l {0}
run: |
cd ${GITHUB_WORKSPACE}/..
git clone https://github.com/jupyter/qtconsole.git
cd qtconsole
${pythonLocation}/bin/python -m pip install -e ".[test]"
${pythonLocation}/bin/python -m pip install pyqt5
- name: Install Jupyter-Client changes
shell: bash -l {0}
run: ${pythonLocation}/bin/python -m pip install -e .

- name: Test qtconsole
shell: bash -l {0}
run: |
cd ${GITHUB_WORKSPACE}/../qtconsole
xvfb-run --auto-servernum ${pythonLocation}/bin/python -m pytest -x -vv -s --full-trace --color=yes qtconsole

downstream_check: # This job does nothing and is only used for the branch protection
if: always()
needs:
Expand All @@ -92,6 +127,7 @@ jobs:
- jupyter_client
- ipyparallel
- jupyter_kernel_test
- qtconsole
runs-on: ubuntu-latest
steps:
- name: Decide whether the needed jobs succeeded or failed
Expand Down
16 changes: 10 additions & 6 deletions ipykernel/comm/comm.py
Expand Up @@ -42,7 +42,7 @@ def publish_msg(self, msg_type, data=None, metadata=None, buffers=None, **keys):


# but for backwards compatibility, we need to inherit from LoggingConfigurable
class Comm(traitlets.config.LoggingConfigurable, BaseComm):
class Comm(BaseComm, traitlets.config.LoggingConfigurable):
"""Class for communicating between a Frontend and a Kernel"""

kernel = Instance("ipykernel.kernelbase.Kernel", allow_none=True) # type:ignore[assignment]
Expand All @@ -68,12 +68,16 @@ def _default_kernel(self):
def _default_comm_id(self):
return uuid.uuid4().hex

def __init__(self, *args, **kwargs):
# Comm takes positional arguments, LoggingConfigurable does not, so we explicitly forward arguments
def __init__(self, target_name='', data=None, metadata=None, buffers=None, **kwargs):
# Handle differing arguments between base classes.
kernel = kwargs.pop('kernel', None)
if target_name:
kwargs['target_name'] = target_name
BaseComm.__init__(
self, data=data, metadata=metadata, buffers=buffers, **kwargs
) # type:ignore[call-arg]
kwargs['kernel'] = kernel
traitlets.config.LoggingConfigurable.__init__(self, **kwargs)
# drop arguments not in BaseComm
kwargs.pop("kernel", None)
BaseComm.__init__(self, *args, **kwargs)


__all__ = ["Comm"]
4 changes: 2 additions & 2 deletions ipykernel/comm/manager.py
Expand Up @@ -9,13 +9,13 @@
import traitlets.config


class CommManager(traitlets.config.LoggingConfigurable, comm.base_comm.CommManager):
class CommManager(comm.base_comm.CommManager, traitlets.config.LoggingConfigurable):

kernel = traitlets.Instance("ipykernel.kernelbase.Kernel")
comms = traitlets.Dict()
targets = traitlets.Dict()

def __init__(self, **kwargs):
# CommManager doesn't take arguments, so we explicitly forward arguments
traitlets.config.LoggingConfigurable.__init__(self, **kwargs)
comm.base_comm.CommManager.__init__(self)
traitlets.config.LoggingConfigurable.__init__(self, **kwargs)
12 changes: 8 additions & 4 deletions ipykernel/tests/test_comm.py
@@ -1,3 +1,5 @@
import unittest.mock

from ipykernel.comm import Comm, CommManager
from ipykernel.ipkernel import IPythonKernel

Expand Down Expand Up @@ -47,10 +49,12 @@ def on_msg(msg):
manager.register_target("fizz", fizz)

kernel.comm_manager = manager
comm = Comm()
comm.on_msg(on_msg)
comm.on_close(on_close)
manager.register_comm(comm)
with unittest.mock.patch.object(Comm, "publish_msg") as publish_msg:
comm = Comm()
comm.on_msg(on_msg)
comm.on_close(on_close)
manager.register_comm(comm)
assert publish_msg.call_count == 1

assert manager.get_comm(comm.comm_id) == comm
assert manager.get_comm('foo') is None
Expand Down