-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
Reliable list_running_servers(runtime_dir) design #5716
Open
abaelhe
wants to merge
18
commits into
jupyter:6.4.x
Choose a base branch
from
abaelhe:master
base: 6.4.x
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 12 commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
bf52746
Reliable list_running_servers(runtime_dir) design
abaelhe 7ae97ff
both TCP and UNIX socket are works.
abaelhe 9cbf95f
Update notebook/notebookapp.py
abaelhe d774b3e
Update notebook/notebookapp.py
abaelhe ca17f0b
consistent persistent file naming rule
abaelhe a4aae8f
consistent persistent file naming rule and same version check rule
abaelhe 40796a6
updated list_running_servers() and write_browser_open_file() ONLY if …
abaelhe 737a963
bind JUPYTER_PORT or random available port
abaelhe 7139ad6
nbserver-%s.json % self.file_id
abaelhe 19210cb
tests update for new connection file naming rule
abaelhe 281966d
Update notebookapp.py
abaelhe 07570c6
Update notebook/notebookapp.py
abaelhe 978d883
Multi-instance and multi-version of Jupyter Notebook
abaelhe 958151a
json.loads(response.body) ---> json.loads(response.body.decode())
abaelhe a305e0d
User Notifications for policy change of list_running_servers()
abaelhe 908a630
User Notifications for policy change of list_running_servers()
abaelhe 57330ff
let user make choices to their data, preference or environment.
abaelhe 40850c4
better message to users
abaelhe File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method should be renamed since its unrelated to kernel. Perhaps
server_request
?Also, please remove the default value from
path
since that should be explicitly provided by the caller (which is already the case anyway).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we creator have this naming right;
with respect to Guido name Python; and their first creator name IPython and Jupyter also Anaconda;
i am working on my IPython Swift Kernel, kernel in my brain;
don't mind, different think lead sparks.
caller will provide their; this default value does not conflict to different
path
;we use '/login' as 'default value' to remind caller authentication first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method needs to change and the default for
path
either removed or updated to/api
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I agree with @kevin-bates here. This method needs to be renamed to something like
api_request
before we can accept it. It's not requesting a response from a "kernel" (as we define it in the Jupyter ecosystem). It's just a general server API ping.I also agree, the default path should probably be
/api
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
who create new who own naming rights.
answered.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name matters here. It obscures this library's API if function names are not clear.
kernel_request
is the wrong name for this function. In the Jupyter ecosystem, kernel refers specifically to the underlying language kernel being executed by calls to the server'skernel
API. This method is not making a kernel request. It's making a generic request the server's REST API. Let's generalize this function name to something likeserver_request
orapi_request
.particularly because Jupyter has a notion of kernels that is specific.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm setting this conversation to unresolved, since this change is necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.