Check existence of connection_file before writing #1127
Merged
Conversation
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
|
Hi @fecet, I think the right fix is to make |
|
But why we have to write this again? Or can we just read the kernel file if exists, and only update the difference? I want this because of jupyter/jupyter_client#953 |
|
Hmm, I think there is a deeper problem then if we're tying to write the connection file from two different kernel managers in practice. That guard is there to prevent a single kernel manager from overwriting its own file. |
I'm a normal jupyter user so may lack many background, but I found the kernelapp is actually overwritting the connection file written by jupyter_client? Also the kernel_name written by jupyter_client seems used CONDA_PREFIX (still don't know why though) |
|
Can you please give the output of |
|
|
So when I use jupyter notebook (v7) open a notebook, Serverapp will write connection_file twice, first comes form https://github.com/jupyter/jupyter_client/blob/57b11c36b1eceae138cfc6044a2274e2e1caa135/jupyter_client/provisioning/local_provisioner.py#L190 |
|
Ah, okay, I think I understand now. Jupyter Client is the primary owner of this file in most cases, but sometimes you might have called |
blink1073
approved these changes
Jun 23, 2023
blink1073
requested changes
Jun 23, 2023
blink1073
reviewed
Jun 23, 2023
blink1073
approved these changes
Jun 23, 2023
blink1073
changed the title
jasongrout
added a commit
that referenced
this pull request
Jul 20, 2023
Before ipykernel 6.23.3, i.e., before #1127, a kernel manager could specify a channel port of 0, and ipykernel would pick a random port and rewrite the connection file with the actual port used. This provided a nice way to address the natural race condition between a kernel manager picking a port and ipykernel actually connecting to it and using it. This unit test tests that this port 0 connection file behavior works, and also tests that existing information in the connection file is not overwritten.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
I find the connection file
kernel-<uuid>.jsonwould be created twice when starting and connecting kernel. I thought this is not as expected since the first time kernel created:https://github.com/jupyter/jupyter_client/blob/57b11c36b1eceae138cfc6044a2274e2e1caa135/jupyter_client/connect.py#L488
do check this. But the second call, i.e., come from the kernelapp doesn't. Thus that json file would overwrite the previous one. I don't know the general order though, this order is I observed when open a new jupyter notebook.
And this time
kernelnamewas not passed. So I never findkernelnamein connection file, I believe this was a bug.