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

Add async API #506

Merged
merged 6 commits into from
Feb 12, 2020
Merged

Add async API #506

merged 6 commits into from
Feb 12, 2020

Conversation

davidbrochart
Copy link
Member

No description provided.

@davidbrochart
Copy link
Member Author

This allows to have asynchronous execution in jupyter/nbclient#10.

Copy link
Contributor

@MSeal MSeal left a comment

Choose a reason for hiding this comment

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

Thanks for working on such a functional PR. Overall the changes seem like a fairly transparent clone of the synchronous client. I'd like to see this be merged and released as it'd help with a major gap in jupyter's python execution framework. While I don't think I'd block merging this to get things moving forward, we probably need tests for the new async paths to help ensure it doesn't break in the future.

That being said, I started helping with this project after these changes were in master but releases were only happening out of the 5.x branch (for a couple years now). @takluyver @Carreau @rgbkrk @mpacer @kevin-bates I might need buy in from you all on what was the state / intention with master branch and if that code should move to an experimental branch instead of master? The two branches are heavily deviated and I haven't worked through what the actual diff encompasses yet.

jupyter_client/asynchronous/channels.py Outdated Show resolved Hide resolved
jupyter_client/asynchronous/client.py Outdated Show resolved Hide resolved
jupyter_client/asynchronous/client.py Show resolved Hide resolved
jupyter_client/asynchronous/client.py Outdated Show resolved Hide resolved
jupyter_client/asynchronous/client.py Outdated Show resolved Hide resolved
@davidbrochart
Copy link
Member Author

Thanks for reviewing @MSeal, I made some changes according to your comments.

@SylvainCorlay
Copy link
Member

this looks good to me.

@SylvainCorlay
Copy link
Member

Everyone, I am going to merge this unless someone comments in the next few hours.

@maartenbreddels

@davidbrochart
Copy link
Member Author

@SylvainCorlay OK to merge?

@SylvainCorlay SylvainCorlay merged commit 284fea0 into jupyter:master Feb 12, 2020
@SylvainCorlay
Copy link
Member

Done!

@rgbkrk
Copy link
Member

rgbkrk commented Feb 17, 2020

Did folks come to a resolution on 5.x vs. master? Excited to see this work, I'd like to use it 🔜 .

@davidbrochart
Copy link
Member Author

Not really (see #511). I opened the same PR in 5.x (#512). But someone will have to reconcile 5.x and master eventually.

@davidbrochart
Copy link
Member Author

I created a PR to forward-port 5.x into master: #513

@meeseeksmachine
Copy link

This pull request has been mentioned on Jupyter Community Forum. There might be relevant details there:

https://discourse.jupyter.org/t/jupyter-client-6-0-0/3414/1

@meeseeksmachine
Copy link

This pull request has been mentioned on Jupyter Community Forum. There might be relevant details there:

https://discourse.jupyter.org/t/welcome-new-jupyter-client-nbclient-maintainer-david-brochart/3467/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants