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.

Sign up for GitHub

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

Jump to bottom

Rework wait_for_ready logic #578

Merged
merged 3 commits into from
Jan 11, 2021
Merged

Rework wait_for_ready logic #578

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
45115b8
Rework wait_for_ready logic
davidbrochart Jan 11, 2021
61fb170
Pin jedi<=0.17.2 in tests
davidbrochart Jan 11, 2021
a6a68b6
Fix tests
davidbrochart Jan 11, 2021
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
19 changes: 15 additions & 4 deletions ipykernel/inprocess/blocking.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,21 @@ class BlockingInProcessKernelClient(InProcessKernelClient):
def wait_for_ready(self):
# Wait for kernel info reply on shell channel
while True:
msg = self.shell_channel.get_msg(block=True)
if msg['msg_type'] == 'kernel_info_reply':
self._handle_kernel_info_reply(msg)
break
self.kernel_info()
try:
msg = self.shell_channel.get_msg(block=True, timeout=1)
except Empty:
pass
else:
if msg['msg_type'] == 'kernel_info_reply':
# Checking that IOPub is connected. If it is not connected, start over.
try:
self.iopub_channel.get_msg(block=True, timeout=0.2)
except Empty:
pass
else:
self._handle_kernel_info_reply(msg)
break

# Flush IOPub channel
while True:
Expand Down
13 changes: 7 additions & 6 deletions ipykernel/tests/test_kernel.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

from .utils import (
new_kernel, kernel, TIMEOUT, assemble_output, execute,
flush_channels, wait_for_idle,
flush_channels, wait_for_idle, get_reply,
)


Expand Down Expand Up @@ -360,9 +360,9 @@ def test_interrupt_during_input():
msg_id = kc.execute("input()")
time.sleep(1) # Make sure it's actually waiting for input.
km.interrupt_kernel()
# If we failed to interrupt interrupt, this will timeout:
reply = kc.get_shell_msg(timeout=TIMEOUT)
from .test_message_spec import validate_message
# If we failed to interrupt interrupt, this will timeout:
reply = get_reply(kc, msg_id, TIMEOUT)
validate_message(reply, 'execute_reply', msg_id)


Expand All @@ -385,9 +385,10 @@ def test_interrupt_during_pdb_set_trace():
msg_id2 = kc.execute("3 + 4")
time.sleep(1) # Make sure it's actually waiting for input.
km.interrupt_kernel()
# If we failed to interrupt interrupt, this will timeout:
from .test_message_spec import validate_message
reply = kc.get_shell_msg(timeout=TIMEOUT)
# If we failed to interrupt interrupt, this will timeout:
reply = get_reply(kc, msg_id, TIMEOUT)
validate_message(reply, 'execute_reply', msg_id)
reply = kc.get_shell_msg(timeout=TIMEOUT)
# If we failed to interrupt interrupt, this will timeout:
reply = get_reply(kc, msg_id2, TIMEOUT)
validate_message(reply, 'execute_reply', msg_id2)
29 changes: 15 additions & 14 deletions ipykernel/tests/test_message_spec.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@
HasTraits, TraitError, Bool, Unicode, Dict, Integer, List, Enum
)

from .utils import TIMEOUT, start_global_kernel, flush_channels, execute
from .utils import (TIMEOUT, start_global_kernel, flush_channels, execute,
get_reply, )

#-----------------------------------------------------------------------------
# Globals
Expand Down Expand Up @@ -278,7 +279,7 @@ def test_execute():
flush_channels()

msg_id = KC.execute(code='x=1')
reply = KC.get_shell_msg(timeout=TIMEOUT)
reply = get_reply(KC, msg_id, TIMEOUT)
validate_message(reply, 'execute_reply', msg_id)


Expand Down Expand Up @@ -405,7 +406,7 @@ def test_oinfo():
flush_channels()

msg_id = KC.inspect('a')
reply = KC.get_shell_msg(timeout=TIMEOUT)
reply = get_reply(KC, msg_id, TIMEOUT)
validate_message(reply, 'inspect_reply', msg_id)


Expand All @@ -415,7 +416,7 @@ def test_oinfo_found():
msg_id, reply = execute(code='a=5')

msg_id = KC.inspect('a')
reply = KC.get_shell_msg(timeout=TIMEOUT)
reply = get_reply(KC, msg_id, TIMEOUT)
validate_message(reply, 'inspect_reply', msg_id)
content = reply['content']
assert content['found']
Expand All @@ -430,7 +431,7 @@ def test_oinfo_detail():
msg_id, reply = execute(code='ip=get_ipython()')

msg_id = KC.inspect('ip.object_inspect', cursor_pos=10, detail_level=1)
reply = KC.get_shell_msg(timeout=TIMEOUT)
reply = get_reply(KC, msg_id, TIMEOUT)
validate_message(reply, 'inspect_reply', msg_id)
content = reply['content']
assert content['found']
Expand All @@ -443,7 +444,7 @@ def test_oinfo_not_found():
flush_channels()

msg_id = KC.inspect('dne')
reply = KC.get_shell_msg(timeout=TIMEOUT)
reply = get_reply(KC, msg_id, TIMEOUT)
validate_message(reply, 'inspect_reply', msg_id)
content = reply['content']
assert not content['found']
Expand All @@ -455,7 +456,7 @@ def test_complete():
msg_id, reply = execute(code="alpha = albert = 5")

msg_id = KC.complete('al', 2)
reply = KC.get_shell_msg(timeout=TIMEOUT)
reply = get_reply(KC, msg_id, TIMEOUT)
validate_message(reply, 'complete_reply', msg_id)
matches = reply['content']['matches']
for name in ('alpha', 'albert'):
Expand All @@ -466,7 +467,7 @@ def test_kernel_info_request():
flush_channels()

msg_id = KC.kernel_info()
reply = KC.get_shell_msg(timeout=TIMEOUT)
reply = get_reply(KC, msg_id, TIMEOUT)
validate_message(reply, 'kernel_info_reply', msg_id)


Expand All @@ -477,7 +478,7 @@ def test_connect_request():
return msg['header']['msg_id']

msg_id = KC.kernel_info()
reply = KC.get_shell_msg(timeout=TIMEOUT)
reply = get_reply(KC, msg_id, TIMEOUT)
validate_message(reply, 'connect_reply', msg_id)


Expand All @@ -486,7 +487,7 @@ def test_comm_info_request():
if not hasattr(KC, 'comm_info'):
raise SkipTest()
msg_id = KC.comm_info()
reply = KC.get_shell_msg(timeout=TIMEOUT)
reply = get_reply(KC, msg_id, TIMEOUT)
validate_message(reply, 'comm_info_reply', msg_id)


Expand All @@ -512,7 +513,7 @@ def test_is_complete():
flush_channels()

msg_id = KC.is_complete("a = 1")
reply = KC.get_shell_msg(timeout=TIMEOUT)
reply = get_reply(KC, msg_id, TIMEOUT)
validate_message(reply, 'is_complete_reply', msg_id)

def test_history_range():
Expand All @@ -522,7 +523,7 @@ def test_history_range():
reply_exec = KC.get_shell_msg(timeout=TIMEOUT)

msg_id = KC.history(hist_access_type = 'range', raw = True, output = True, start = 1, stop = 2, session = 0)
reply = KC.get_shell_msg(timeout=TIMEOUT)
reply = get_reply(KC, msg_id, TIMEOUT)
validate_message(reply, 'history_reply', msg_id)
content = reply['content']
assert len(content['history']) == 1
Expand All @@ -534,7 +535,7 @@ def test_history_tail():
reply_exec = KC.get_shell_msg(timeout=TIMEOUT)

msg_id = KC.history(hist_access_type = 'tail', raw = True, output = True, n = 1, session = 0)
reply = KC.get_shell_msg(timeout=TIMEOUT)
reply = get_reply(KC, msg_id, TIMEOUT)
validate_message(reply, 'history_reply', msg_id)
content = reply['content']
assert len(content['history']) == 1
Expand All @@ -546,7 +547,7 @@ def test_history_search():
reply_exec = KC.get_shell_msg(timeout=TIMEOUT)

msg_id = KC.history(hist_access_type = 'search', raw = True, output = True, n = 1, pattern = '*', session = 0)
reply = KC.get_shell_msg(timeout=TIMEOUT)
reply = get_reply(KC, msg_id, TIMEOUT)
validate_message(reply, 'history_reply', msg_id)
content = reply['content']
assert len(content['history']) == 1
Expand Down
16 changes: 15 additions & 1 deletion ipykernel/tests/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import atexit
import os
import sys
from time import time

from contextlib import contextmanager
from queue import Empty
Expand Down Expand Up @@ -52,13 +53,26 @@ def flush_channels(kc=None):
validate_message(msg)


def get_reply(kc, msg_id, timeout):
timeout = TIMEOUT
t0 = time()
while True:
reply = kc.get_shell_msg(timeout=timeout)
if reply['parent_header']['msg_id'] == msg_id:
break
t1 = time()
timeout -= t1 - t0
t0 = t1
return reply


def execute(code='', kc=None, **kwargs):
"""wrapper for doing common steps for validating an execution request"""
from .test_message_spec import validate_message
if kc is None:
kc = KC
msg_id = kc.execute(code=code, **kwargs)
reply = kc.get_shell_msg(timeout=TIMEOUT)
reply = get_reply(kc, msg_id, TIMEOUT)
validate_message(reply, 'execute_reply', msg_id)
busy = kc.get_iopub_msg(timeout=TIMEOUT)
validate_message(busy, 'status', msg_id)
Expand Down
1 change: 1 addition & 0 deletions setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ def run(self):
'pytest-cov',
'flaky',
'nose', # nose because there are still a few nose.tools imports hanging around
'jedi<=0.17.2'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs to be fixed in IPython, not here. Pinging @bollwyvl and @Carreau about it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eventually yes, but the tests were failing without that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm aware of this, but I'm swamped and haven't had a chance to look at IPython since mid december, I know there are a bunch of issues and PRs I need o review/fix/merge.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davidbrochart - can we add a comment to the effect here, so that we know later when we can remove the pin?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in #579

],
},
classifiers=[
Expand Down