Skip to content
New issue

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

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

Already on GitHub? # to your account

use get_shell_msg helper method #368

Merged
merged 2 commits into from
Aug 24, 2021

Conversation

tschaume
Copy link
Contributor

fixes #367

@@ -101,7 +101,7 @@ def start_kernel(self, *args, **kwargs):
# executed
if self.parent.personality.should_seed_cell(code):
client.execute(code)
msg = client.shell_channel.get_msg(block=True)
msg = client.get_shell_msg(block=True)
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 the block=True needs to be removed.

@davidbrochart - Should we update the migration guide to indicate what calls using block=True should do since we only address the block=False case wrt to a timeout value? I found removing the block=True to be successful but am not entirely sure.

Choose a reason for hiding this comment

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

block=True was the default value, so it could have been omitted previously. It translates to a value of timeout>0 now, which is the default value, so there's nothing to do, and block=True just has to be removed. We can clarify this in the migration guide, and also state that using e.g. client.get_shell_msg() should be used instead of client.shell_channel.get_msg(). I opened jupyter/jupyter_client#685.

@kevin-bates
Copy link
Member

Hi @tschaume - I went ahead and pushed the change to remove the block parameter and the tests now pass.

Once this is merged, I'd like to cut 2.5.1.

@kevin-bates kevin-bates merged commit 85a4899 into jupyter-server:master Aug 24, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

block kwarg removed in jupyter-client 7
3 participants