Skip to content

Conversation

@kevin-bates
Copy link
Collaborator

Update to use the kernel-id generated by the provider. If not set we'll go ahead and assign an id anyway. If we decide to raise an issue (or log a warning), that's fine as well. This approach seemed the least intrusive.

@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (jupyter-kernel-mgmt@9d3c7e0). Click here to learn what that means.
The diff coverage is 66.66%.

Impacted file tree graph

@@                  Coverage Diff                   @@
##             jupyter-kernel-mgmt       #3   +/-   ##
======================================================
  Coverage                       ?   73.63%           
======================================================
  Files                          ?       99           
  Lines                          ?     8894           
  Branches                       ?        0           
======================================================
  Hits                           ?     6549           
  Misses                         ?     2345           
  Partials                       ?        0
Impacted Files Coverage Δ
notebook/services/kernels/kernelmanager.py 78.1% <66.66%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9d3c7e0...ab84f2a. Read the comment docs.

@takluyver
Copy link
Owner

Seems reasonable. You may want to make your own PR against notebook so you're not relying on me merging things into my PR.

@takluyver takluyver merged commit 54fdb14 into takluyver:jupyter-kernel-mgmt Jul 22, 2019
@kevin-bates
Copy link
Collaborator Author

Good idea regarding making my own PR. I'll do that. (However, I had just added another update to pin tornado to 5.1.1.) I can make that part of my PR if you'd rather we split this now.

@takluyver
Copy link
Owner

Or I can add you to my notebook fork so you can update the branch, if you prefer. Splitting's not the aim, just avoiding me being the bottleneck.

@kevin-bates
Copy link
Collaborator Author

That's fine too. I appreciate that. I just don't want to get going down the wrong road in the single branch either. That said, I suppose just having one WIP PR for the same thing is probably less confusing to the Notebook maintainers.

Please go ahead and add me to your branch. If there's a change that's too radical, I'll submit a PR to it instead. Does that sound ok?

@takluyver
Copy link
Owner

That sounds fine. I've added you.

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.

3 participants