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

Delete ctrl if associated kernel is deleted #12148

Merged
merged 9 commits into from
Nov 23, 2022

Conversation

DonJayamanne
Copy link
Contributor

No description provided.

@@ -302,7 +341,7 @@ export class ControllerRegistration implements IControllerRegistration {
// TODO: Don't hide controllers that are already associated with a notebook.
// If we have a notebook opened and its using a kernel.
// Else we end up killing the execution as well.
if (this.isFiltered(item.connection) && !this.isControllerAttachedToADocument(item)) {
if (this.isFiltered(item.connection) && this.canControllerBeDisposed(item)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was incorrect, wonder how many users use the old filter UI, cant wait for that to be removed
too much stuff to manage due to kernel filters

@DonJayamanne DonJayamanne force-pushed the deleteDeletedActiveIntepreter branch from 59ef208 to 737ae42 Compare November 23, 2022 04:27
@DonJayamanne DonJayamanne mentioned this pull request Nov 23, 2022
54 tasks
@codecov-commenter
Copy link

codecov-commenter commented Nov 23, 2022

Codecov Report

Merging #12148 (19ca6f9) into main (55ca7ac) will decrease coverage by 0%.
The diff coverage is 88%.

@@           Coverage Diff           @@
##            main   #12148    +/-   ##
=======================================
- Coverage     70%      70%    -1%     
=======================================
  Files        497      497            
  Lines      31276    31384   +108     
  Branches    4907     4922    +15     
=======================================
+ Hits       21970    22034    +64     
- Misses      7438     7470    +32     
- Partials    1868     1880    +12     
Impacted Files Coverage Δ
src/kernels/internalTypes.ts 100% <ø> (ø)
src/platform/interpreter/contracts.ts 100% <ø> (ø)
.../raw/finder/localKnownPathKernelSpecFinder.node.ts 73% <68%> (-3%) ⬇️
...rc/notebooks/controllers/controllerRegistration.ts 84% <80%> (-3%) ⬇️
src/platform/api/pythonApi.ts 70% <90%> (-1%) ⬇️
...aw/finder/contributedLocalKernelSpecFinder.node.ts 83% <92%> (-4%) ⬇️
...raw/finder/contributedLocalPythonEnvFinder.node.ts 87% <94%> (+<1%) ⬆️
src/kernels/jupyter/finder/remoteKernelFinder.ts 87% <100%> (+<1%) ⬆️
...lPythonAndRelatedNonPythonKernelSpecFinder.node.ts 72% <100%> (+1%) ⬆️
...tebooks/controllers/kernelSource/kernelSelector.ts 73% <100%> (-7%) ⬇️
... and 17 more

@DonJayamanne DonJayamanne marked this pull request as ready for review November 23, 2022 04:52
@DonJayamanne DonJayamanne enabled auto-merge (squash) November 23, 2022 04:52
@DonJayamanne DonJayamanne marked this pull request as draft November 23, 2022 06:20
auto-merge was automatically disabled November 23, 2022 06:20

Pull request was converted to draft

@DonJayamanne DonJayamanne changed the title Delete ctrl if associated active env is deleted Delete ctrl if associated env is deleted Nov 23, 2022
@DonJayamanne DonJayamanne marked this pull request as ready for review November 23, 2022 21:36
@DonJayamanne DonJayamanne enabled auto-merge (squash) November 23, 2022 21:36
@DonJayamanne DonJayamanne changed the title Delete ctrl if associated env is deleted Delete ctrl if associated kernel is deleted Nov 23, 2022
@DonJayamanne DonJayamanne merged commit 7e54651 into main Nov 23, 2022
@DonJayamanne DonJayamanne deleted the deleteDeletedActiveIntepreter branch November 23, 2022 22:15
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