Merge pull request #7432 from minrk/load-race

use promises to wait for kernelspecs on notebook load

IPython/html/static/notebook/js/kernelselector.js

@@ -9,6 +9,7 b' define(['
     "use strict";
     
     var KernelSelector = function(selector, notebook) {
+        var that = this;
         this.selector = selector;
         this.notebook = notebook;
         this.notebook.set_kernelselector(this);
@@ -22,6 +23,11 b' define(['
         this.bind_events();
         // Make the object globally available for user convenience & inspection
         IPython.kernelselector = this;
+        this._finish_load = null;
+        this.loaded = new Promise(function(resolve, reject) {
+            that._finish_load = resolve;
+        });
+        
         Object.seal(this);
     };
     
@@ -47,26 +53,29 b' define(['
                 return -1;
             }
         });
-
+        
+        var i, ks, ks_submenu_entry;
         // Create the Kernel > Change kernel submenu
-        for (var i = 0; i < keys.length; i++) {
-            var ks = this.kernelspecs[keys[i]];
-            var ks_submenu_entry = $("<li>").attr("id", "kernel-submenu-"+ks.name).append($('<a>')
+        for (i = 0; i < keys.length; i++) {
+            ks = this.kernelspecs[keys[i]];
+            ks_submenu_entry = $("<li>").attr("id", "kernel-submenu-"+ks.name).append($('<a>')
                 .attr('href', '#')
-                .click($.proxy(this.change_kernel, this, ks.name))
+                .click($.proxy(this.set_kernel, this, ks.name))
                 .text(ks.spec.display_name));
             change_kernel_submenu.append(ks_submenu_entry);
         }
         
         // Create the File > New Notebook submenu
-        for (var i = 0; i < keys.length; i++) {
-            var ks = this.kernelspecs[keys[i]];
-            var ks_submenu_entry = $("<li>").attr("id", "new-notebook-submenu-"+ks.name).append($('<a>')
+        for (i = 0; i < keys.length; i++) {
+            ks = this.kernelspecs[keys[i]];
+            ks_submenu_entry = $("<li>").attr("id", "new-notebook-submenu-"+ks.name).append($('<a>')
                 .attr('href', '#')
                 .click($.proxy(this.new_notebook, this, ks.name))
                 .text(ks.spec.display_name));
             new_notebook_submenu.append(ks_submenu_entry);
         }
+        // trigger loaded promise
+        this._finish_load();
     };
     
     KernelSelector.prototype._spec_changed = function (event, ks) {
@@ -80,7 +89,7 b' define(['
         if (cur_kernel_entry.length) {
             cur_kernel_entry.parent().prepend($("<li>").attr("class","divider"))
                                      .prepend(cur_kernel_entry);
-        };
+        }
         
         // load logo
         var logo_img = this.element.find("img.current_kernel_logo");
@@ -118,28 +127,26 b' define(['
         }
     };
 
-    KernelSelector.prototype.change_kernel = function (kernel_name) {
-        /**
-         * TODO, have a methods to set kernel spec directly ?
-         **/
+    KernelSelector.prototype.set_kernel = function (kernel_name) {
+        /** set the kernel by name, ensuring kernelspecs have been loaded, first */
+        var that = this;
+        return this.loaded.then(function () {
+            that._set_kernel(kernel_name);
+        });
+    };
+
+    KernelSelector.prototype._set_kernel = function (kernel_name) {
+        /** Actually set the kernel (kernelspecs have been loaded) */
         if (kernel_name === this.current_selection) {
+            // only trigger event if value changed
             return;
         }
         var ks = this.kernelspecs[kernel_name];
-        
-        try {
-            this.notebook.start_session(kernel_name);
-        } catch (e) {
-            if (e.name === 'SessionAlreadyStarting') {
-                console.log("Cannot change kernel while waiting for pending session start.");
-            } else {
-                // unhandled error
-                throw e;
-            }
-            // only trigger spec_changed if change was successful
+        if (this.notebook._session_starting) {
+            console.error("Cannot change kernel while waiting for pending session start.");
             return;
         }
-        console.log('spec', kernel_name, ks);
+        this.current_selection = kernel_name;
         this.events.trigger('spec_changed.Kernel', ks);
     };
 
@@ -181,13 +188,7 b' define(['
         this.events.on('spec_changed.Kernel', $.proxy(this._spec_changed, this));
 
         this.events.on('kernel_created.Session', function (event, data) {
-            if (data.kernel.name !== that.current_selection) {
-                // If we created a 'python' session, we only know if it's Python
-                // 3 or 2 on the server's reply, so we fire the event again to
-                // set things up.
-                var ks = that.kernelspecs[data.kernel.name];
-                that.events.trigger('spec_changed.Kernel', ks);
-            }
+            that.set_kernel(data.kernel.name);
         });
         
         var logo_img = this.element.find("img.current_kernel_logo");

IPython/html/static/notebook/js/notebook.js

@@ -248,6 +248,10 b' define(['
         this.events.on('spec_changed.Kernel', function(event, data) {
             that.metadata.kernelspec = 
                 {name: data.name, display_name: data.spec.display_name};
+            // start session if the current session isn't already correct
+            if (!(this.session && this.session.kernel && this.session.kernel.name === data.name)) {
+                that.start_session(data.name);
+            }
         });
 
         this.events.on('kernel_ready.Kernel', function(event, data) {
@@ -1763,19 +1767,6 b' define(['
         this.notebook_path = data.path;
         var trusted = true;
         
-        // Trigger an event changing the kernel spec - this will set the default
-        // codemirror mode
-        if (this.metadata.kernelspec !== undefined) {
-            // TODO shoudl probably not trigger here, 
-            // should call the kernel selector, or custom.{js|css} not loaded.
-            if(this.kernel_selector){
-                // technically not perfect, we should check that the kernelspec matches
-                this.kernel_selector.change_kernel(this.metadata.kernelspec.name);
-            } else {
-                console.log('do not have handle on kernel_selector');
-            }
-        }
-        
         // Set the codemirror mode from language_info metadata
         if (this.metadata.language_info !== undefined) {
             var langinfo = this.metadata.language_info;
@@ -2077,6 +2068,7 b' define(['
      * @param {string} notebook_path - A notebook to load
      */
     Notebook.prototype.load_notebook = function (notebook_path) {
+        var that = this;
         this.notebook_path = notebook_path;
         this.notebook_name = utils.url_path_split(this.notebook_path)[1];
         this.events.trigger('notebook_loading.Notebook');
@@ -2192,8 +2184,6 b' define(['
             this.nbformat_minor = nbmodel.nbformat_minor;
         }
         
-        // Create the session after the notebook is completely loaded to prevent
-        // code execution upon loading, which is a security risk.
         if (this.session === null) {
             var kernel_name;
             if (this.metadata.kernelspec) {
@@ -2202,7 +2192,14 b' define(['
             } else {
                 kernel_name = utils.get_url_param('kernel_name');
             }
-            this.start_session(kernel_name);
+            if (kernel_name) {
+                // setting kernel_name here triggers start_session
+                this.kernel_selector.set_kernel(kernel_name);
+            } else {
+                // start a new session with the server's default kernel
+                // spec_changed events will fire after kernel is loaded
+                this.start_session();
+            }
         }
         // load our checkpoint list
         this.list_checkpoints();