Skip to content

Modernize the Server #23

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

Closed
wants to merge 29 commits into from
Closed

Conversation

blink1073
Copy link
Contributor

@blink1073 blink1073 commented Aug 9, 2018

  • Switch to Python 3.5+ so we can use async/await
  • Use async/await instead of gen.coroutine
  • Switch to pytest
  • Remove ipython_genutils
  • Some PEP8 changes along the way

To view the diff without whitespace changes: https://github.com/jupyter/jupyter_server/pull/23/files?w=1

@blink1073
Copy link
Contributor Author

blink1073 commented Aug 9, 2018

@minrk, of note, see the use of force_async in place of gen.maybe_future, which is deprecated and didn't get along with async/await.

@blink1073
Copy link
Contributor Author

And of course, 12/26 commits come from CI problems...

@blink1073
Copy link
Contributor Author

blink1073 commented Aug 10, 2018

Annoyingly, we had to wrap every use of yield with force_async, because unlike modern JS, Python won't consume a non-awaitable object. This works in JS: let foo = await 1; console.log(foo), who knew it would start being better than Python 😉?

@blink1073
Copy link
Contributor Author

Submitted an issue to the python bug tracker: https://bugs.python.org/issue34380

@minrk
Copy link
Contributor

minrk commented Sep 17, 2018

Raising when you try to await something you can't await seems like a feature, not a bug to me ;)

This is really great! I think we need to think pretty carefully about the transition, as this repo needs to be very proactively porting every PR that lands in jupyter/notebook in order to avoid divergence problems. Modernizing the code like this is super nice, but it will also make that transition period a whole lot harder by guaranteeing that ~all patches need manual changes in order to apply cleanly.

@blink1073
Copy link
Contributor Author

blink1073 commented Sep 17, 2018

Solid points Min, let's close this...

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.

2 participants