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

Use async_start_new_kernel_client from nbclient #597

Merged
merged 7 commits into from
Jun 15, 2020
Merged

Use async_start_new_kernel_client from nbclient #597

merged 7 commits into from
Jun 15, 2020

Conversation

davidbrochart
Copy link
Member

Once nbclient 0.3.0 is released this will be ready to be merged.

@davidbrochart
Copy link
Member Author

Trigger CI.

@davidbrochart davidbrochart reopened this May 14, 2020
@davidbrochart
Copy link
Member Author

Not sure why tests/app/many_iopub_messages_test.py fails on OSX/python3.8 in Travis.

@maartenbreddels
Copy link
Member

This probably: WARNING traitlets:client.py:550 Timeout waiting for IOPub output we need to configure nbclient to not timeout (i would suggest 240 second like the pytest argument)

@davidbrochart
Copy link
Member Author

Trigger CI.

@davidbrochart davidbrochart reopened this May 19, 2020
@SylvainCorlay
Copy link
Member

Rebased.

@SylvainCorlay SylvainCorlay mentioned this pull request Jun 2, 2020
Closed
SylvainCorlay
SylvainCorlay reviewed Jun 2, 2020
View reviewed changes
share/jupyter/voila/templates/lab/index.html.j2 Outdated
@@ -46,7 +46,7 @@ var voila_process = function(cell_index, cell_count) {
Therefore it is important to have the cell loop in the template.
The issue for Jinja is: https://github.com/pallets/jinja/issues/1044
#}
{%- for cell in cell_generator(nb, kernel_id) -%}
{%- for cell in cell_generator(nb) -%}
Copy link
Member

Choose a reason for hiding this comment

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

So we don't need the kernel id in the jupyter-config-data?

I recall there was a comment by @maartenbreddels about this, but don't remember what.

Copy link
Member Author

Choose a reason for hiding this comment

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

jupyter-config-data gets the kernel_id from kernel_start().

Copy link
Member

Choose a reason for hiding this comment

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

Deja vu feeling here :), I think it was a different PR. We do need the kernel_id, otherwise, we break the API, and we lose flexibility.

Copy link
Member Author

Choose a reason for hiding this comment

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

👍

@davidbrochart
Copy link
Member Author

Let's merge this PR.

@SylvainCorlay SylvainCorlay merged commit 20dd249 into voila-dashboards:master Jun 15, 2020
@davidbrochart davidbrochart deleted the nbclient branch June 15, 2020 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maartenbreddels maartenbreddels maartenbreddels left review comments

@SylvainCorlay SylvainCorlay SylvainCorlay left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@davidbrochart @maartenbreddels @SylvainCorlay