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

async cells are sometimes executed out of order #16

Closed
saraedum opened this issue Oct 7, 2021 · 3 comments · Fixed by #17
Closed

async cells are sometimes executed out of order #16

saraedum opened this issue Oct 7, 2021 · 3 comments · Fixed by #17
Labels
bug Something isn't working

Comments

@saraedum
Copy link
Contributor

saraedum commented Oct 7, 2021

Running all cells in this notebook (with Cell → Run All) executes cells out of order:

image

This happened with the latest version of this package and the latest notebook from conda-forge.

I am happy to provide more details if that is helpful.

@saraedum
Copy link
Contributor Author

saraedum commented Oct 7, 2021

This is likely because I am on ipykernel 5.5.5.

@saraedum
Copy link
Contributor Author

saraedum commented Oct 7, 2021

I don't claim to fully understand what I am doing here but this fixes the issue for me:

diff --git a/jupyter_ui_poll/_poll.py b/jupyter_ui_poll/_poll.py
index 81fb11e..3f80957 100644
--- a/jupyter_ui_poll/_poll.py
+++ b/jupyter_ui_poll/_poll.py
@@ -89,9 +89,7 @@ class KernelWrapper:
             if kernel._aborting:
                 kernel._send_abort_reply(stream, parent, ident)
             else:
-                rr = kernel.execute_request(stream, ident, parent)
-                if self._kernel_is_async:
-                    await rr
+                await kernel.execute_request(stream, ident, parent)

                 # replicate shell_dispatch behaviour
                 sys.stdout.flush()

@Kirill888
Copy link
Owner

Thanks for the report @saraedum , I can't say I have tested async + older ipykernel, it could be that kernel.execute_request returns awaitable when operating in async mode, completely disabling that check is not an option though as this would break synchronous case on series 5.* of ipykernel.

Code here can be improved though

rr = kernel.execute_request(stream, ident, parent)
if self._kernel_is_async:
await rr

Probably most robust is to check whether rr is "awaitable", and then await, instead of relying on per-kernel check.

@Kirill888 Kirill888 added the bug Something isn't working label Oct 8, 2021
saraedum added a commit to saraedum/jupyter-ui-poll that referenced this issue Oct 8, 2021
Fixes Kirill888#16.

In some cases ipykernel 5 returns an awaitable when executing a request.
We need to await it to get the correct order of executing of async
cells.
saraedum added a commit to saraedum/jupyter-ui-poll that referenced this issue Oct 8, 2021
Fixes Kirill888#16.

In some cases ipykernel 5 returns an awaitable when executing a request.
We need to await it to get the correct order of execution of async cells.
Kirill888 pushed a commit that referenced this issue Oct 8, 2021
Fixes #16.

In some cases ipykernel 5 returns an awaitable when executing a request.
We need to await it to get the correct order of execution of async cells.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants