Allow connection to kernels by files #847

Merged
merged 17 commits into from Oct 13, 2011

Projects

None yet

3 participants

@minrk
IPython member

This moves all the connection info for a kernel into JSON files like the parallel code, so there's no need to specify 4 ports to connect to an existing kernel. In fact, if you just specify ipython qtconsole --existing, it will try to connect to the most recent kernel, including those started by the notebook.

It also cleans up some KernelManager code in the notebook, so that changes to the base KernelManager will be inherited in the notebook. The main immediate benefit being that restarting a notebook kernel really does restart it, so attached qtconsoles will remain attached.

I will also turn HMAC signatures on by default before merging.

This should address #688, #806, and even probably #691.

@minrk
IPython member

HMAC signatures are now on by default in all situations, including notebook, kernel, and qtconsole.

@minrk
IPython member

Also add %connect_info magic, to make it easy to connect secondary clients.

For reference, this function may be useful to folks like @fperez, who like to connect a qtconsole to their current notebook:

def connect_qtconsole():
    from subprocess import Popen, PIPE
    from IPython.zmq.kernelapp import KernelApp
    app = KernelApp.instance()
    return Popen(['ipython', 'qtconsole', '--existing', app.connection_file])

Which, when called, will launch a qtconsole connected to the kernel that called it. This applies to kernels started with a notebook, other qtconsole, etc.

@fperez
IPython member

@minrk, that's great, and I'd actually vote for wrapping it in a magic also, that we expose interactively for all kernels. Havig the function api is useful for scripted control, but users would simply type %qtconsole. I love the usability this would provide!

@minrk
IPython member

%qtconsole magic added

@minrk
IPython member

@ivanov can you check if any of the changes here cause problems for you with vim-ipython? That's the only third party client I know of. The change of ip/port pairs to just ports might mean you have to make an adjustment, but in the long run it should be simpler, because you shouldn't have to keep track of ports at all.

@fperez
IPython member

@minrk, thanks a lot for the magic! I think this is a major usability improvement, awesome work. Will wait for @ivanov's feedback before moving further.

@fperez
IPython member

Just to say, %qtconsole is awesome. Using it now in production work, it rocks.

@ivanov
IPython member

looking into it now...

@minrk
IPython member

I peeked into vim-ipython a bit, and it shouldn't be a big deal. You just need to specify one argument: BlockingKM(connection_file=/path/to/kernel-12345.json) instead of trying to parse the argv. The check for whether this is available will be:

if 'connection_file' in BlockingKernelManager.class_trait_names():
    # 0.12
    km = BlockingKernelManager(connection_file=s)
else:
    # parse line as before for 0.11
@minrk
IPython member

It did require a small change to be convenient, but I think it should be working now: ivanov/vim-ipython#13

@minrk
IPython member

Just a note: this will certainly conflict with #813 in qtconsoleapp.py, so let's pick merge order carefully, as that one has already been rebased at least once.

@ivanov
IPython member

as usual, Min fixed the issues i was going to complain about, even before I had the chance to verbalize the complaint :)

My only concern is related to profiles, since, as I understand it - clients aren't apps, but the connection_file get stored in the active profile's security/ folder. I suppose it's not a big issue, since the connection_info magic prints the files contents

@minrk
IPython member
@ivanov
IPython member

Note that you can always specify the full path of the connection file, regardless of profiles, etc.

yes, true, but you have to work a bit to get that full path, since it isn't what's printed, but that's ok.

The profile security dir is just the default, and used if only a filename is given.

right - so i guess that's the trade off - before, i didn't have to know anything about profiles, just hookup to the right ports and go. Now it seems I have to look in the profile directory in order to get to the file (though, i suppose, there's nothing stopping me from using the contents of the file as printed by the connection_info magic in the same manner as i used to in 0.11)

@minrk
IPython member

Note that you can always specify the full path of the connection file, regardless of profiles, etc.

yes, true, but you have to work a bit to get that full path, since it isn't what's printed, but that's ok.>> The profile security dir is just the default, and used if only a filename is given.

The full path is only not printed if it's in the default location, so if the output is just --existing kernel-12345.json, that unambiguously means IPYTHONDIR/profile_default/security/kernel-12345.json. It will also print the profile name if it is anything other than default (--existing kernel-12345.json --profile foo), and it will print the full path in all other cases.

right - so i guess that's the trade off - before, i didn't have to know anything about profiles, just hookup to the right ports and go. Now it seems I have to look in the profile directory in order to get to the file (though, i suppose, there's nothing stopping me from using the contents of the file as printed by the connection_info magic in the same manner as i used to in 0.11)

Yes, it's a tradeoff. An alternate approach would be if I made the KernelManager itself profile-aware (it's not currently), which would mean that clients using the KernelManager would generally not need to be. An example of the KernelManager not being profile aware - if you create a KernelManager unconfigured and use it to start a Kernel, the connection file will go in a temp dir:

>>> from IPython.zmq.blockingkernelmanager import BlockingKernelManager
>>> km = BlockingKernelManager()
>>> km.start_kernel()
[IPKernelApp] To connect another client to this kernel, use:
[IPKernelApp] --existing /var/folders/zz/v3fz17cd2nz0bqvpd2s7btq00000gn/T/tmpMBJMP6.json
>>> km.start_channels()

So it's only non-IPython apps that want to start clients connected to IPython-started kernels under default config that are at any disadvantage.

We could also add an API like:

get_security_file(fname, profile='default'):
    """returns security file from IPython profile directory"""
    return os.path.join(get_ipython_dir(), 'profile_%s' % profile, 'security', fname)

So that apps would not need to know about our directory structure, only the more abstract idea of profiles.

@ivanov
IPython member
@minrk
IPython member

get_security_file() added to utils.path

@fperez fperez commented on the diff Oct 13, 2011
IPython/frontend/qt/console/qtconsoleapp.py
self.kernel_argv.remove(a)
+ if len(split) == 1:
+ # alias passed with arg via space
+ swallow_next = True
+
+ def init_connection_file(self):
@fperez
fperez Oct 13, 2011

A small docstring here summarizing what this puppy does would be good...

@fperez fperez commented on the diff Oct 13, 2011
IPython/frontend/qt/console/qtconsoleapp.py
# Create a KernelManager and start a kernel.
self.kernel_manager = QtKernelManager(
- shell_address=(self.ip, self.shell_port),
- sub_address=(self.ip, self.iopub_port),
- stdin_address=(self.ip, self.stdin_port),
- hb_address=(self.ip, self.hb_port),
- config=self.config
+ ip=self.ip,
@fperez
fperez Oct 13, 2011

ip = self.ip,, etc.. Spaces around = missing :)

@minrk
minrk Oct 13, 2011

Do we really want to add extra space around = when specifying arguments? We do that approximately nowhere else.

@fperez
fperez Oct 13, 2011
@minrk
minrk Oct 13, 2011
@fperez
fperez Oct 13, 2011
@fperez
IPython member

I only have some trivial cosmetic comments on most of it (made inline), and one of substance on the magics: let's make both magics be trivial wrappers around regular Python library functions. Testing magics is kind of annoying, and using them from other code also awkward, b/c you have to call them via the magic system. And for example, the %connect_info should be split into a function that just builds that info string --but doesn't print it, simply returns it--, and the actual magic to simply do print connection_info(). That makes it easy for other clients who may want to show this same info to collect it as a string from a single location.

Same thing for the %qtconsole, even though it's easy code, it should just be a wrapper around a library function. The point of the magics, as I see them now, is just to isolate a namespace of convenient things, but they should not have much implementation code in them. Historically that hasn't been true (%run is a monster), but we should move towards that model. Ultimately, I'd like the magics to be all more or less trivial wrappers around regular library calls.

@fperez
IPython member

Other than the above trivial cosmetic fixes and the slightly more substantial (but easy) fixing of the magics, I'm +100 on merging this ASAP. Unless anyone objects, let's merge it once it's fixed up; the improvements it brings are really major. Fantastic work!

@minrk
IPython member

I did the cleanup on qtconsoleapp.

The library functions make perfect sense. What would you call them, and where would you put them? IPython.lib.kernel? They are general utilities for connecting to a kernel. Should the get_connection_info() function return the unpacked JSON dict, the filename, or the string contents of the file?

minrk added some commits Oct 9, 2011
@minrk minrk KernelManager has port traits instead of multiple ip/port pairs
all foo_address traits became foo_port, adding one 'ip' trait for all

This matches the Kernel, which does not allow specifying multiple
IPs for each channel.
09371f3
@minrk minrk fix kernel_argv scrubbing to cover args passed with space
$> ipqt --shell 12345

would not scrub the port, previously
fc5db52
@minrk minrk use connection files instead of ports to connect to kernels
this addresses security issues (#688)
and ease-of-multi-frontend
ebac3b1
@minrk minrk fix remaining sub_port -> iopub_port in kernelmanager 386003c
@minrk minrk use zmq.KernelManager to manage individual kernels in notebook
This means that proper restart is now available, rather than
killing and starting a new kernel, breaking connections to
secondary frontends.
7dc06b6
@minrk minrk allow reuse of connection file with ssh tunnels 08a42df
@minrk minrk enable HMAC message signing by default in kernels
add session_aliases and session_flags to consolidate these settings

use new consolidated flags in parallel apps as well

add default_secure() function for use in apps where the default
behavior should be to enable HMAC signatures.
8555f5b
@minrk minrk enable HMAC message signing by default in notebook kernels
This is separate from previous, because it is more likely to be rejected.

It requires the the Session objects in Handlers get a reference all the way back up to the IPython App that started the environment.
203fd1a
@minrk minrk add %connect_info magic for help connecting secondary clients 7d350bb
@minrk minrk add %qtconsole magic for conveniently launching second console 02be235
@minrk minrk add KernelManager.load_connection_file method
for loading connection info from its connection_file
c3f5dd3
@minrk minrk py3compat pass on Session.key
ensure str_to_bytes and bytes_to_str are used to transform Session.key to/from JSON
d1cd0af
@minrk minrk add get_security_file() function to utils.path
This makes it easier for third party tools to retrieve security files
(e.g. json connection files) by name and [optional] profile name alone, without
knowledge of IPython's directory structure.

For example, whenever ipkernel outputs:

` --existing kernel-12345.json --profile foo`

that file can be found with:

get_security_file('kernel-12345.json', profile='foo')
2c75ec8
@ivanov
IPython member

not sure if i'm OT, but I'd like to also see init_ssh functionality grafted out of qtconsoleapp and placed wherever other general utilities go

@minrk
IPython member

@ivanov argh! That's a good idea. I will never be done with this.

@minrk
IPython member

@ivanov, tunnel_to_kernel should now be a generic function for tunneling connections to a kernel, given a connection file or its unpacked contents.

@fperez
IPython member
minrk added some commits Oct 13, 2011
@minrk minrk cleanup pass on qtconsoleap per review
principally adding big docstring to complicated init_ssh method
73cd6ea
@minrk minrk add IPython.lib.kernel
get_connection_info and connect_qtconsole are now implemented as library functions
in the new module.  The zmqshell magics that call them are now simple wrappers.
171c7ae
@minrk minrk split QtConsole.init_ssh into generic tunnel_to_kernel
added to new IPython.lib.kernel with other kernel connection utilities.
46eb4e1
@minrk
IPython member

I also split the connection file search into a library function, so that will be easier to reuse as well.

@minrk minrk split qtconsole's connection-file search into lib.kernel
* fileglob-based searching for connection file added as
  IPython.lib.kernel.find_connection_file()

* also use this search in other lib.kernel methods, to make them
  more useful when run outside an IPython kernel.
48450ef
@fperez
IPython member

@minrk, this is looking pretty solid, and big enough we shouldn't let it linger much further. I can't think of much more to do on it at this point, can you? Otherwise, let's push it through. Awesome job!

@minrk
IPython member

I'm just playing with various use cases to make sure it works, then I'll go ahead and merge.

@ivanov, I also updated the vim-ipython PR, to use the new glob-based file search

@minrk minrk added a commit that referenced this pull request Oct 13, 2011
@minrk minrk Merge PR #847 (connection files)
* JSON connection files are now used to connect files
* HMAC message signing is now on by default in all IPython apps
* Adds IPython.lib.kernel, which contains utility functions for connecting
  clients. These were mostly copied out of qtconsoleapp in order to be
  more general.
* Adds %connection_info and %qtconsole magics to zmqshell

closes gh-688
closes gh-806
closes gh-691
f93a8ca
@minrk minrk merged commit 48450ef into ipython:master Oct 13, 2011
@minrk
IPython member

merged

@mattvonrocketstein mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
@minrk minrk Merge PR #847 (connection files)
* JSON connection files are now used to connect files
* HMAC message signing is now on by default in all IPython apps
* Adds IPython.lib.kernel, which contains utility functions for connecting
  clients. These were mostly copied out of qtconsoleapp in order to be
  more general.
* Adds %connection_info and %qtconsole magics to zmqshell

closes gh-688
closes gh-806
closes gh-691
906733a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment