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

create epic for launching a jupyter ws kernel #2391

Merged
merged 2 commits into from
Jan 23, 2018

Conversation

rgbkrk
Copy link
Member

@rgbkrk rgbkrk commented Jan 17, 2018

This sets up a kernel launching epic that launches kernels on jupyter hosts.

🎉 Connections to remote kernels, ready for desktop, play, and the nteract web app. Connecting this up to Desktop is not a focus for this PR, nor is the UI/config for doing so. The first target is the jupyter extension.

TODO:

  • Match semantics of the zeromq kernel launch
  • Adapt either the jupyter extension or rx-jupyter to adapt the URL based on location.origin as necessary
  • Integrate into the jupyter extension app
  • Tests

@rgbkrk rgbkrk force-pushed the jupyter-kernels-epic branch from 21f76c7 to 7347a82 Compare January 17, 2018 23:15
@rgbkrk
Copy link
Member Author

rgbkrk commented Jan 19, 2018

One thing that rx-jupyter doesn't do that it probably should -- if it's not passed the full URL for the endpoint it should construct the full URL (especially for the websockets) using location.origin and the baseURI. Another slight annoyance is the different names we have for some of the same bits:

  • baseUrl - our current jupyter extension base URI (e.g. /user/fvzhyd)
  • serverUrl - what we've set up for host records
  • serverConfig.endpoint || serverConfig.url same thing in rx-jupyter, which is used as the full base URL https://hub.example.org/user/fvzhyd yet can also function as the pure base path on a localhost server /user/fvzhyd but not in all cases (notably websockets)

Thoughts @theengineear? We mostly just need consistency among these while being able to straddle localhost (HTTP) and remote authenticated servers on HTTPS.

@rgbkrk
Copy link
Member Author

rgbkrk commented Jan 19, 2018

Notes from the jupyter notebook server as to why the jupyter extension used baseURL -- the server itself uses baseURL for that base URI.

@theengineear
Copy link
Contributor

@rgbkrk

if it's not passed the full URL for the endpoint it should construct the full URL

Is there any reason for us not to required that users of the lib just pass in the full URL, else we just bail in rx-jupyter?

@rgbkrk
Copy link
Member Author

rgbkrk commented Jan 20, 2018

Is there any reason for us not to required that users of the lib just pass in the full URL, else we just bail in rx-jupyter?

I think that's a good standard.

@rgbkrk rgbkrk force-pushed the jupyter-kernels-epic branch 2 times, most recently from 06afff4 to 130948d Compare January 22, 2018 17:46
@rgbkrk
Copy link
Member Author

rgbkrk commented Jan 22, 2018

Rebased this one to stay up to date, going to town on finishing it off now.

@rgbkrk rgbkrk force-pushed the jupyter-kernels-epic branch 3 times, most recently from 93ccb96 to dedae0f Compare January 23, 2018 16:34
@rgbkrk
Copy link
Member Author

rgbkrk commented Jan 23, 2018

The way I'm running this right now (since we don't have the contents API and SET_NOTEBOOK integrations setup) is firing off this action with the redux devtools:

{
  type: 'LAUNCH_KERNEL_BY_NAME',
  kernelSpecName: 'python3',
  cwd: '/'
}

@rgbkrk rgbkrk force-pushed the jupyter-kernels-epic branch 2 times, most recently from ec85256 to 417fdf5 Compare January 23, 2018 18:34
@rgbkrk
Copy link
Member Author

rgbkrk commented Jan 23, 2018

💥

@rgbkrk rgbkrk force-pushed the jupyter-kernels-epic branch from 417fdf5 to 26d67e5 Compare January 23, 2018 18:43
@rgbkrk rgbkrk force-pushed the jupyter-kernels-epic branch from 26d67e5 to b3b84e1 Compare January 23, 2018 18:45
@rgbkrk
Copy link
Member Author

rgbkrk commented Jan 23, 2018

This now also sets the kernel into the app record. Execution works as does autocomplete. 😄

@rgbkrk
Copy link
Member Author

rgbkrk commented Jan 23, 2018

execution

Copy link
Contributor

@theengineear theengineear left a comment

Choose a reason for hiding this comment

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

🎉

@theengineear theengineear merged commit 6c44d63 into nteract:master Jan 23, 2018
@rgbkrk rgbkrk deleted the jupyter-kernels-epic branch January 23, 2018 21:41
@lock
Copy link

lock bot commented May 1, 2018

Howdy! I'm 🔓🤖!

In order to keep information timely (based on the most recent release), we want all activity to be added to either new issues or open issues and PRs. In service to that goal, I, the lock bot close inactive closed issues when they haven't had activity in 120 days.

Feel free to open a new issue for related bugs and link to relevant comments from this thread.

@lock lock bot locked and limited conversation to collaborators May 1, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants