Merge pull request #2811 from minrk/ipc_defaults

Still more KernelManager cleanup

Finish up some more work from  #2775

closes  #2722

- [x] ip/transport configurables only live on KernelManager objects, not ConsoleApp
- [x] fix default ipc paths in `ipython kernel` and `ipython notebook` cases
- [x] fix ipc file cleanup in a few more cases
- [x] add `--transport` alias for all appropriate entry points

IPython/frontend/consoleapp.py

@@ -88,11 +88,12 b' aliases = dict(ipkernel_aliases)'
 
 # also scrub aliases from the frontend
 app_aliases = dict(
+    ip = 'KernelManager.ip',
+    transport = 'KernelManager.transport',
     hb = 'IPythonConsoleApp.hb_port',
     shell = 'IPythonConsoleApp.shell_port',
     iopub = 'IPythonConsoleApp.iopub_port',
     stdin = 'IPythonConsoleApp.stdin_port',
-    ip = 'IPythonConsoleApp.ip',
     existing = 'IPythonConsoleApp.existing',
     f = 'IPythonConsoleApp.connection_file',
 
@@ -153,27 +154,6 b' class IPythonConsoleApp(Configurable):'
     auto_create = CBool(True)
     # connection info:
     
-    transport = CaselessStrEnum(['tcp', 'ipc'], default_value='tcp', config=True)
-    
-    ip = Unicode(config=True,
-        help="""Set the kernel\'s IP address [default localhost].
-        If the IP address is something other than localhost, then
-        Consoles on other machines will be able to connect
-        to the Kernel, so be careful!"""
-    )
-    def _ip_default(self):
-        if self.transport == 'tcp':
-            return LOCALHOST
-        else:
-            # this can fire early if ip is given,
-            # in which case our return value is meaningless
-            if not hasattr(self, 'profile_dir'):
-                return ''
-            ipcdir = os.path.join(self.profile_dir.security_dir, 'kernel-%s' % os.getpid())
-            os.makedirs(ipcdir)
-            atexit.register(lambda : shutil.rmtree(ipcdir))
-            return os.path.join(ipcdir, 'ipc')
-    
     sshserver = Unicode('', config=True,
         help="""The SSH server to use to connect to the kernel.""")
     sshkey = Unicode('', config=True,
@@ -274,9 +254,9 b' class IPythonConsoleApp(Configurable):'
         with open(fname) as f:
             cfg = json.load(f)
         
-        self.transport = cfg.get('transport', 'tcp')
-        if 'ip' in cfg:
-            self.ip = cfg['ip']
+        self.config.KernelManager.transport = cfg.get('transport', 'tcp')
+        self.config.KernelManager.ip = cfg.get('ip', LOCALHOST)
+        
         for channel in ('hb', 'shell', 'iopub', 'stdin'):
             name = channel + '_port'
             if getattr(self, name) == 0 and name in cfg:
@@ -284,34 +264,38 b' class IPythonConsoleApp(Configurable):'
                 setattr(self, name, cfg[name])
         if 'key' in cfg:
             self.config.Session.key = str_to_bytes(cfg['key'])
-        
     
     def init_ssh(self):
         """set up ssh tunnels, if needed."""
-        if not self.sshserver and not self.sshkey:
+        if not self.existing or (not self.sshserver and not self.sshkey):
             return
         
-        if self.transport != 'tcp':
-            self.log.error("Can only use ssh tunnels with TCP sockets, not %s", self.transport)
-            return
+        self.load_connection_file()
+        
+        transport = self.config.KernelManager.transport
+        ip = self.config.KernelManager.ip
+        
+        if transport != 'tcp':
+            self.log.error("Can only use ssh tunnels with TCP sockets, not %s", transport)
+            sys.exit(-1)
         
         if self.sshkey and not self.sshserver:
             # specifying just the key implies that we are connecting directly
-            self.sshserver = self.ip
-            self.ip = LOCALHOST
+            self.sshserver = ip
+            ip = LOCALHOST
         
         # build connection dict for tunnels:
-        info = dict(ip=self.ip,
+        info = dict(ip=ip,
                     shell_port=self.shell_port,
                     iopub_port=self.iopub_port,
                     stdin_port=self.stdin_port,
                     hb_port=self.hb_port
         )
         
-        self.log.info("Forwarding connections to %s via %s"%(self.ip, self.sshserver))
+        self.log.info("Forwarding connections to %s via %s"%(ip, self.sshserver))
         
         # tunnels return a new set of ports, which will be on localhost:
-        self.ip = LOCALHOST
+        self.config.KernelManager.ip = LOCALHOST
         try:
             newports = tunnel_to_kernel(info, self.sshserver, self.sshkey)
         except:
@@ -347,8 +331,6 b' class IPythonConsoleApp(Configurable):'
 
         # Create a KernelManager and start a kernel.
         self.kernel_manager = self.kernel_manager_class(
-                                transport=self.transport,
-                                ip=self.ip,
                                 shell_port=self.shell_port,
                                 iopub_port=self.iopub_port,
                                 stdin_port=self.stdin_port,
@@ -359,6 +341,7 b' class IPythonConsoleApp(Configurable):'
         # start the kernel
         if not self.existing:
             self.kernel_manager.start_kernel(extra_arguments=self.kernel_argv)
+            atexit.register(self.kernel_manager.cleanup_ipc_files)
         elif self.sshserver:
             # ssh, write new connection file
             self.kernel_manager.write_connection_file()

IPython/frontend/html/notebook/notebookapp.py

@@ -242,6 +242,7 b' aliases.update({'
     'ip': 'NotebookApp.ip',
     'port': 'NotebookApp.port',
     'port-retries': 'NotebookApp.port_retries',
+    'transport': 'KernelManager.transport',
     'keyfile': 'NotebookApp.keyfile',
     'certfile': 'NotebookApp.certfile',
     'notebook-dir': 'NotebookManager.notebook_dir',

IPython/frontend/qt/console/qtconsoleapp.py

@@ -193,9 +193,7 b' class IPythonQtConsoleApp(BaseIPythonApplication, IPythonConsoleApp):'
     def new_frontend_master(self):
         """ Create and return new frontend attached to new kernel, launched on localhost.
         """
-        ip = self.ip if self.ip in LOCAL_IPS else LOCALHOST
         kernel_manager = self.kernel_manager_class(
-                                ip=ip,
                                 connection_file=self._new_connection_file(),
                                 config=self.config,
         )
@@ -245,7 +243,11 b' class IPythonQtConsoleApp(BaseIPythonApplication, IPythonConsoleApp):'
         self.app.icon = QtGui.QIcon(icon_path)
         QtGui.QApplication.setWindowIcon(self.app.icon)
 
-        local_kernel = (not self.existing) or self.ip in LOCAL_IPS
+        try:
+            ip = self.config.KernelManager.ip
+        except AttributeError:
+            ip = LOCALHOST
+        local_kernel = (not self.existing) or ip in LOCAL_IPS
         self.widget = self.widget_factory(config=self.config,
                                         local_kernel=local_kernel)
         self.init_colors(self.widget)

IPython/zmq/heartbeat.py

@@ -41,7 +41,8 b' class Heartbeat(Thread):'
                 self.port = s.getsockname()[1]
                 s.close()
             elif addr[0] == 'ipc':
-                while os.path.exists(self.ip + '-' + self.port):
+                self.port = 1
+                while os.path.exists("%s-%s" % (self.ip, self.port)):
                     self.port = self.port + 1
             else:
                 raise ValueError("Unrecognized zmq transport: %s" % addr[0])

IPython/zmq/kernelapp.py

@@ -62,6 +62,7 b' kernel_aliases.update({'
     'stdin' : 'KernelApp.stdin_port',
     'f' : 'KernelApp.connection_file',
     'parent': 'KernelApp.parent',
+    'transport': 'KernelApp.transport',
 })
 if sys.platform.startswith('win'):
     kernel_aliases['interrupt'] = 'KernelApp.interrupt'
@@ -98,7 +99,6 b' class KernelApp(BaseIPythonApplication):'
     heartbeat = Instance(Heartbeat)
     session = Instance('IPython.zmq.session.Session')
     ports = Dict()
-    _full_connection_file = Unicode()
     
     # inherit config file name from parent:
     parent_appname = Unicode(config=True)
@@ -112,8 +112,16 b' class KernelApp(BaseIPythonApplication):'
         
     # connection info:
     transport = CaselessStrEnum(['tcp', 'ipc'], default_value='tcp', config=True)
-    ip = Unicode(LOCALHOST, config=True,
+    ip = Unicode(config=True,
         help="Set the IP or interface on which the kernel will listen.")
+    def _ip_default(self):
+        if self.transport == 'ipc':
+            if self.connection_file:
+                return os.path.splitext(self.abs_connection_file)[0] + '-ipc'
+            else:
+                return 'kernel-ipc'
+        else:
+            return LOCALHOST
     hb_port = Integer(0, config=True, help="set the heartbeat port [default: random]")
     shell_port = Integer(0, config=True, help="set the shell (ROUTER) port [default: random]")
     iopub_port = Integer(0, config=True, help="set the iopub (PUB) port [default: random]")
@@ -122,9 +130,16 b' class KernelApp(BaseIPythonApplication):'
     help="""JSON file in which to store connection info [default: kernel-<pid>.json]
     
     This file will contain the IP, ports, and authentication key needed to connect
-    clients to this kernel. By default, this file will be created in the security-dir
+    clients to this kernel. By default, this file will be created in the security dir
     of the current profile, but can be specified by absolute path.
     """)
+    @property
+    def abs_connection_file(self):
+        if os.path.basename(self.connection_file) == self.connection_file:
+            return os.path.join(self.profile_dir.security_dir, self.connection_file)
+        else:
+            return self.connection_file
+        
 
     # streams, etc.
     no_stdout = Bool(False, config=True, help="redirect stdout to the null device")
@@ -141,7 +156,7 b' class KernelApp(BaseIPythonApplication):'
         """)
     interrupt = Integer(0, config=True,
         help="""ONLY USED ON WINDOWS
-        Interrupt this process when the parent is signalled.
+        Interrupt this process when the parent is signaled.
         """)
 
     def init_crash_handler(self):
@@ -158,11 +173,21 b' class KernelApp(BaseIPythonApplication):'
 
     def _bind_socket(self, s, port):
         iface = '%s://%s' % (self.transport, self.ip)
-        if port <= 0 and self.transport == 'tcp':
-            port = s.bind_to_random_port(iface)
-        else:
-            c = ':' if self.transport == 'tcp' else '-'
-            s.bind(iface + c + str(port))
+        if self.transport == 'tcp':
+            if port <= 0:
+                port = s.bind_to_random_port(iface)
+            else:
+                s.bind("tcp://%s:%i" % (self.ip, port))
+        elif self.transport == 'ipc':
+            if port <= 0:
+                port = 1
+                path = "%s-%i" % (self.ip, port)
+                while os.path.exists(path):
+                    port = port + 1
+                    path = "%s-%i" % (self.ip, port)
+            else:
+                path = "%s-%i" % (self.ip, port)
+            s.bind("ipc://%s" % path)
         return port
 
     def load_connection_file(self):
@@ -179,7 +204,7 b' class KernelApp(BaseIPythonApplication):'
             s = f.read()
         cfg = json.loads(s)
         self.transport = cfg.get('transport', self.transport)
-        if self.ip == LOCALHOST and 'ip' in cfg:
+        if self.ip == self._ip_default() and 'ip' in cfg:
             # not overridden by config or cl_args
             self.ip = cfg['ip']
         for channel in ('hb', 'shell', 'iopub', 'stdin'):
@@ -192,19 +217,15 b' class KernelApp(BaseIPythonApplication):'
     
     def write_connection_file(self):
         """write connection info to JSON file"""
-        if os.path.basename(self.connection_file) == self.connection_file:
-            cf = os.path.join(self.profile_dir.security_dir, self.connection_file)
-        else:
-            cf = self.connection_file
+        cf = self.abs_connection_file
+        self.log.debug("Writing connection file: %s", cf)
         write_connection_file(cf, ip=self.ip, key=self.session.key, transport=self.transport,
         shell_port=self.shell_port, stdin_port=self.stdin_port, hb_port=self.hb_port,
         iopub_port=self.iopub_port)
-        
-        self._full_connection_file = cf
     
     def cleanup_connection_file(self):
-        cf = self._full_connection_file
-        self.log.debug("cleaning up connection file: %r", cf)
+        cf = self.abs_connection_file
+        self.log.debug("Cleaning up connection file: %s", cf)
         try:
             os.remove(cf)
         except (IOError, OSError):

IPython/zmq/kernelmanager.py

@@ -684,7 +684,20 b' class KernelManager(Configurable):'
     
     transport = CaselessStrEnum(['tcp', 'ipc'], default_value='tcp', config=True)
 
-    ip = Unicode(LOCALHOST, config=True)
+    ip = Unicode(LOCALHOST, config=True,
+        help="""Set the kernel\'s IP address [default localhost].
+        If the IP address is something other than localhost, then
+        Consoles on other machines will be able to connect
+        to the Kernel, so be careful!"""
+    )
+    def _ip_default(self):
+        if self.transport == 'ipc':
+            if self.connection_file:
+                return os.path.splitext(self.connection_file)[0] + '-ipc'
+            else:
+                return 'kernel-ipc'
+        else:
+            return LOCALHOST
     def _ip_changed(self, name, old, new):
         if new == '*':
             self.ip = '0.0.0.0'
@@ -706,8 +719,8 b' class KernelManager(Configurable):'
     _stdin_channel = Any
     _hb_channel = Any
     _connection_file_written=Bool(False)
- 	
-    def __del__(self):	  	
+
+    def __del__(self):
         self.cleanup_connection_file()
 
     #--------------------------------------------------------------------------