Two-process terminal frontend (ipython core branch) #864
Conversation
|
Thanks for pounding on this one! BTW, I'd like to change the name of the client from |
|
thanks, great!!! |
|
I think I'd given it the name |
|
My reasoning was:
But at least that gives a clear pattern that should be easy for users to understand/remember. |
|
Progress report so far:
|
|
awesome! I'll look through it later. For flags&aliases, I recommend looking at IPython.core.shellapp, and its two derivatives: IPKernelApp and TerminalIPythonApp. That mixin is actually less complete than what you put together, only defining the unique elements, and letting the descendants assemble the various components. I don't know which approach is better. It would be great if you can split the swallow_args into just a function - it really should just be a single function with three args: since it's generically useful for any app that starts one or more others in a subprocess, but you want to be able to specify command-line args for both (e.g. I have designs on using it in ipcluster, so that some flags can be passed on to ipcontroller/ipengine). I don't exactly know where that should go, though. Maybe utils.process? |
| swallow_next = False | ||
| was_flag = False | ||
| # copy again, in case some aliases have the same name as a flag | ||
| # argv = list(self.kernel_argv) |
There was a problem hiding this comment.
This second copy is a leftover from when there were two passes. This isn't necessary anymore.
(I know, I'm reviewing my own messy code)
|
And if you could figure out how to cleanup connection files on sigint, that would be great. I think you are better at signal-handling than I am. The connection-file cleanup is handled in the |
|
I'm pretty sure I've figured out the connection file cleanup, I'll push that shortly, unless you are working on it. |
|
okay, connection file cleanup is definitely better now. ctrl-C killed qtconsole will cleanup all of its files, and the terminal app will also get it done under normal circumstances. Obviously unclean shutdown (term) doesn't allow for cleanup. |
|
flags and aliases are working now, as well. The main things that remain are:
|
|
Getting pretty close. I just fixed a few things regarding various message cases, signaling, startup, and unresponsive kernels, and moved the new code into frontend.terminal.console. Another thing that needs to be done: port the multiline history changes in shell.interact, once #929 is merged. I did have to fix two things outside the new code:
|
|
update on recent changes:
Still todo:
after IRC discussion with @fperez, the idea is: frontend magics can be accessed exclusively as For instance, this could be handled with Simple
|
|
Here's a quick stab at the right regexp for this: import re
names = ['kernel', 'edit']
pattern = '^%(?P<name>' + '|'.join(names) + ') (?P<args>.*\S)\s+'
magic_re = re.compile(pattern)
line = '%kernel a1 a2 '
m = magic_re.match(line)
print 'pattern:', pattern
if m:
print 'name:', m.group('name')
print 'args:', m.group('args')We'll also need to add the logic to catch ? and ??, but that should be about it. |
|
I fixed a couple of small problems arising from my renaming of lib.pylabtools, but I'm still getting an error in the test suite: I looked at that file, and there's nothing named |
|
In actual testing, I noticed that |
|
@fperez - I think I see that you pushed some changes to ipython/origin-termzmq. Did you mean to push them here? |
|
On Mon, Nov 28, 2011 at 12:10 PM, Min RK
Ah, OK. But the code is still looking for it, so that's an error...
Yes, thanks for catching that. But now I realized that's a bad idea, |
|
I just did a rebase/push without issue. Also removed the unused frontend.py. |
|
Thanks! But I'm really puzzled, I tried again on a different machine and got the same set of errors: Furthermore, the above looks like a git bug: that index.lock file didn't exist before, I made sure to remove it... And even if I remove it again, it bombs even on Very strange... In any case, thanks! I'll keep a close eye on this version of git, in case it turns out I have to file a git bug report (for reference, it's git 1.7.5.4 from ubuntu 11.10). Back to ipython... The test suite almost passes, but I'm getting these really strange errors: Do you see those too? They're in |
|
Ah, I used to see those - I don't know why they happen, but it seems like it tries to do something multithreaded, and can hit a race condition when rebasing many commits. I always As for iptest errors - no, I do not see any errors, the test suite passes just fine. |
|
On Mon, Nov 28, 2011 at 9:55 PM, Min RK
OK, good to know. I'll use that trick from now on.
That's really odd... OK, I'll worry about it later then. It seems like the big bug left here is output flushing when raw_input |
|
The %kernel frontend-magic stuff hasn't been implemented, but I've been thinking, and in my experience, most of the time that I want to kill the kernel, I want to do it when it's not responding, and a magic doesn't help. Other than that, I can't think of anything else that jumped out. |
|
@fperez - Do we want to block on the frontend-magics? Note that with the design we have proposed, the magics will only function if the frontend believes the kernel to be fully functional and waiting for input. That seems to completely eliminate the value of I don't think it's particularly useful to be able to signal the kernel only when it's perceived to be fully functional and responsive. |
adds frontend_flags/aliases traits to ConsoleApp derivatives for use in this function.
* allow EOF to be forwarded * flush iopub in more appropriate places * handle interrupting raw_input better
|
Mmh, something broke here, I typed Does the console work for you? This tells me that we should, at the very least, add a test that starts a console, runs something simple, and shuts it down. We could do it using pexpect, actually... |
|
Of course, this was the first rebase since the PromptManager. Should be a quick fix. |
|
PromptManager issue fixed, I'll add a simple test. |
|
simple pexpect test added as well |
|
A few things:
to
|
|
Interesting, because I see none of those issues. The KeyError I know is a bug that just doesn't happen to come up for me, but I'll fix it. However, it is strange that you have incorrect prompt spacing and exit doesn't work, when they behave perfectly for me. Do you have any related config? |
|
The KeyError should be fixed, but I don't know how to deal with the others since I can't reproduce them. |
|
Ok, that's truly bizarre: no special config, just tried by making a clean, fresh profile and running with that one. Still see all the above problems. |
|
Confirming that your last commit fixes the KeyError, now I can start with |
|
I just tried it on Linux, and now I do see both. I wonder how they would be Linux-specific? |
|
Beats me, that's really odd. But at least it's not just little green men on my box :) |
|
No, I am somehow completely crazy. It's new since yesterday after the rebase, so I was using the old version. The prompt separation should be fixed now. I don't know why exit would stop working. Any idea? |
|
Yup, prompts look fine now. No clue on the exit not working, b/c |
|
I don't know how exit stopped being handled, but it should work now. The main remaining issue is that the SIGINT catching machinery doesn't really work when a GUI is integrated (only affects owning frontend, not |
|
Well, everything is looking good enough to merge. In your question above about the magics, did you mean whether we should block merging this until we sort that out, or whether the frontend should block on frontend magics? |
|
The former - should we block merge |
|
I think we can merge now, and sort out what we want for Regarding But precisely since it's small polish, let's worry about it post-merge. The rest of this is too useful and important not to have it in now. I'm merging it; a huge thanks to you, @ivanov, @omazapa and everyone else who worked on this! @omazapa, you see how it took a long time but finally your work is in, and it will be part of the new 0.12 release :) |
Two-process terminal frontend: this branch adds a new IPython frontend, invoked via ipython console that behaves much like the regular, old ipython, but runs over zeromq in two processes. This means that such a client can connect to existing kernels initiated by the Qt console, the notebook or standalone (i.e. via `ipython kernel`). We still have some internal architectural cleanups to perform to simplify how the various frontends talk to the kernels, but by having this main piece in, the complete picture is clearer, and that refactoring work can be carried post-0.12. This frontend should still be considered experimental.
Two-process terminal frontend: this branch adds a new IPython frontend, invoked via ipython console that behaves much like the regular, old ipython, but runs over zeromq in two processes. This means that such a client can connect to existing kernels initiated by the Qt console, the notebook or standalone (i.e. via `ipython kernel`). We still have some internal architectural cleanups to perform to simplify how the various frontends talk to the kernels, but by having this main piece in, the complete picture is clearer, and that refactoring work can be carried post-0.12. This frontend should still be considered experimental.
This PR supersedes @minrk's #708 (and in turn, @omazapa's #433)
It's not ready to merge yet, but I've fixed Min's complaint about ctrl-c crashing the kernel.
one thing i see currently wrong is when you invoke
ipython zmq --existing ...- ctrl-c is broken and complains about not having a kernel (now fixed)