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

PR: Shutdown kernel of the notebook before closing it #26

Merged
merged 4 commits into from
Jan 16, 2017
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions spyder_notebook/notebookplugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,24 @@
"""

# Stdlib imports
import os
import os.path as osp
import subprocess
import sys
import tempfile
import json

Copy link
Member

Choose a reason for hiding this comment

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

Remove this blank

# Qt imports
from qtpy.QtWidgets import QApplication, QMessageBox, QVBoxLayout, QMenu
from qtpy.QtCore import Qt, Signal
from qtpy.compat import getsavefilename

# Notebook imports
from notebook.utils import url_path_join

# Third-party imports
import nbformat
import requests

# Spyder imports
from spyder.config.base import _
Expand Down Expand Up @@ -223,13 +230,44 @@ def close_client(self, index=None, client=None):
if index is not None:
client = self.tabwidget.widget(index)

self.shutdown_kernel(client)

# TODO: Eliminate the notebook from disk if it's an Untitled one
client.close()

# Note: notebook index may have changed after closing related widgets
self.tabwidget.removeTab(self.tabwidget.indexOf(client))
self.clients.remove(client)

def shutdown_kernel(self, client):
Copy link
Member

Choose a reason for hiding this comment

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

Please move this method to NotebookClient. It fits better there than here.

"""Shutdown the kernel of the given client."""
sessions_url = url_path_join(client.server_url, 'api/sessions')
sessions_req = requests.get(sessions_url).content.decode()
sessions = json.loads(sessions_req)
kernel_id = None
if os.name == 'nt':
path = client.path.replace('\\', '/')
else:
path = client.path
for session in sessions:
if session['notebook']['path'] == path:
kernel_id = session['kernel']['id']
break
if kernel_id:
delete_url = url_path_join(client.server_url,
'api/kernels/',
kernel_id)
delete_req = requests.delete(delete_url)
if delete_req.status_code != 204:
QMessageBox.warning(
self,
_("Server error"),
_("The Jupyter Notebook server failed "
"to shutdown the kernel. "
"If you want to shutdown it you would "
"have to do it manually."
Copy link
Member

@ccordoba12 ccordoba12 Jan 16, 2017

Choose a reason for hiding this comment

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

Please rewrite this as

The Jupyter Notebook server failed to shutdown the kernel associated with this notebook.
If you want to shut it down, you'll have to close Spyder.

"<br><br>Status code {}").format(delete_req.status_code))
Copy link
Member

Choose a reason for hiding this comment

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

Please remove this line, I don't think we need to show the response's status code.


def save_as(self):
"""Save notebook as..."""
current_client = self.get_current_client()
Expand Down