PR: Shutdown kernel of the notebook before closing it #26
Conversation
| @@ -223,6 +229,29 @@ def close_client(self, index=None, client=None): | |||
| if index is not None: | |||
| client = self.tabwidget.widget(index) | |||
|
|
|||
| session_url = url_path_join(client.server_url, 'api/sessions') | |||
There was a problem hiding this comment.
Let's call this sessions_url
| @@ -223,6 +229,29 @@ def close_client(self, index=None, client=None): | |||
| if index is not None: | |||
| client = self.tabwidget.widget(index) | |||
|
|
|||
| session_url = url_path_join(client.server_url, 'api/sessions') | |||
| session_req = requests.get(session_url).content.decode() | |||
There was a problem hiding this comment.
Change this one to sessions_req
| content = json.loads(session_req) | ||
| kernel_id = None | ||
| for notebook in content: | ||
| if notebook['notebook']['path'] == client.path.replace('\\','/'): |
There was a problem hiding this comment.
The replace part only needs to be applied on Windows.
| kernel_id) | ||
| delete_req = requests.delete(delete_url) | ||
| if delete_req.status_code != 204: | ||
| QMessageBox.critical( |
There was a problem hiding this comment.
Let's change critical here for warning.
|
Great job @dalthviz! I only left some minor comments to improve readability ;-) |
| @@ -223,6 +229,29 @@ def close_client(self, index=None, client=None): | |||
| if index is not None: | |||
| client = self.tabwidget.widget(index) | |||
|
|
|||
| session_url = url_path_join(client.server_url, 'api/sessions') | |||
| session_req = requests.get(session_url).content.decode() | |||
| content = json.loads(session_req) | |||
There was a problem hiding this comment.
This must be sessions = json.loads(session_req)
| session_req = requests.get(session_url).content.decode() | ||
| content = json.loads(session_req) | ||
| kernel_id = None | ||
| for notebook in content: |
There was a problem hiding this comment.
Change this to for session in sessions:
| # 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): |
There was a problem hiding this comment.
Please move this method to NotebookClient. It fits better there than here.
| _("The Jupyter Notebook server failed " | ||
| "to shutdown the kernel. " | ||
| "If you want to shutdown it you would " | ||
| "have to do it manually." |
There was a problem hiding this comment.
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.
| "to shutdown the kernel. " | ||
| "If you want to shutdown it you would " | ||
| "have to do it manually." | ||
| "<br><br>Status code {}").format(delete_req.status_code)) |
There was a problem hiding this comment.
Please remove this line, I don't think we need to show the response's status code.
|
A couple more comments from me and then this will be ready :-) |
…utdown_kernel method.
| @@ -12,6 +12,8 @@ | |||
| import subprocess | |||
| import sys | |||
| import tempfile | |||
|
|
|||
|
|
|||
|
Thanks @dalthviz! |
Fixes #5