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
Kernel status/shutdown from dashboard #1676
Conversation
refresh notebook list and cluster list when : - page get focus - every 60 sec when page is on focus stop refreshing every 60 sec when page loose focus
if kid is not None: | ||
f['kernel_status']=kid | ||
else: | ||
f['kernel_status']='off' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are going to store kernel ID and kernel status in a single key, perhaps it would be clearer to just do:
f['kernel'] = km.kernel_for_notebook(nid)
And then in js, you would check null:
if(kernel){
this.add_shutdown_button(item, kernel);
} else {
this.add_delete_button(item);
}
Also minor style note: try to add spaces around assignment.
This looks pretty good. My only behavioral complaint is what happens when you shutdown a kernel. Frontends are not notified by anything other than regular heart failure. But then again, I think this is exactly what happens if you kill a kernel from one frontend, while it is in use by another, so perhaps coordinating clean shutdown is a separate issue. |
True, I should have done that when changing 'on' to kernel_id. I guess this does not prevent us from the clean shutdown option in the notebook menubar. |
files = nbm.list_notebooks() | ||
for f in files : | ||
nid = f['notebook_id'] | ||
kid = km.kernel_for_notebook(nid) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for this if statement. just use f['kernel_id'] = km.kernel_for_notebook(nid)
. Then use null in js instead of undefined.
set kernel id to None/null if not started
Done. |
don't clear list if 'upload' button are present to avoid clearing the list and the upload form
@@ -590,7 +590,10 @@ class NotebookRootHandler(AuthenticatedHandler): | |||
@authenticate_unless_readonly | |||
def get(self): | |||
nbm = self.application.notebook_manager | |||
km = self.application.kernel_manager |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking a bit more about this, I think this is OK. We do need to try and keep the kernel and notebook server side handlers+logic separate, but we have to have some way for the notebook handler to learn about the kernels for that notebook.
Don't expose enable and disable autorefresh done. |
superseed ipython#1676 ipython#1658 (and correct bug in 1676 where one con't upload notebook because of refresh)
Closing as it is part of Pull Request #1739 now. |
superseed ipython#1676 ipython#1658 (and correct bug in 1676 where one con't upload notebook because of refresh)
Dashboard improvement see ipython#1658 ipython#1676 Allow to shutdown the kernels from the dashboard, autorefresh dashboard, add a native upload method, especially for https/chrome/linux that prevent drag and drop
this is a partial fix for #1515 ,
It replace the
delete
button byshutdown
button when kernel are running.the list is updated when dashboard page get focus, and refresh every 60 seconds.
Refresh stop when page loose focus.
It also move the
clear_list
when clicking on the refresh button to avoid flickering.