Permit kernel std* to be redirected #420

Merged
merged 4 commits into from May 11, 2011

Conversation

Projects
None yet
3 participants
@epatters
Contributor

epatters commented May 9, 2011

I have added support for redirecting a launched kernel's stdin/stdout/stderr. As usual, Windows caused me some hassle.

I also fixed a few related bugs, the most significant of which was causing the heartbeat channel to poll indefinitely sometimes!

- sys.stdout = sys.stderr = blackhole
- sys.__stdout__ = sys.__stderr__ = blackhole
+ if namespace.no_stdout:
+ sys.stdout = sys.__stdout__ = blackhole

This comment has been minimized.

Show comment Hide comment
@takluyver

takluyver May 9, 2011

Member

According to the docs (http://docs.python.org/library/sys.html#sys.__stdin__), the double-underscored versions of the names (sys.__stdout__ etc.) are supposed to preserve the original stdin/stdout/stderr for anything that explicitly wants to access them later. What's our rationale for overriding those here, in addition to the 'working locations' (sys.stdout etc.)?

@takluyver

takluyver May 9, 2011

Member

According to the docs (http://docs.python.org/library/sys.html#sys.__stdin__), the double-underscored versions of the names (sys.__stdout__ etc.) are supposed to preserve the original stdin/stdout/stderr for anything that explicitly wants to access them later. What's our rationale for overriding those here, in addition to the 'working locations' (sys.stdout etc.)?

This comment has been minimized.

Show comment Hide comment
@takluyver

takluyver May 9, 2011

Member

P.S. I recognise that we were already doing that before this pull request. I'm just interested to understand it.

@takluyver

takluyver May 9, 2011

Member

P.S. I recognise that we were already doing that before this pull request. I'm just interested to understand it.

This comment has been minimized.

Show comment Hide comment
@epatters

epatters May 9, 2011

Contributor

The rational is that the sys.__stdout__ and sys.__stderr__ created for pythonw.exe are broken. If you write more than 4096 bytes to them, the interpreter will crash. Nice, huh? There is no reason to preserve them--you don't want them.

@epatters

epatters May 9, 2011

Contributor

The rational is that the sys.__stdout__ and sys.__stderr__ created for pythonw.exe are broken. If you write more than 4096 bytes to them, the interpreter will crash. Nice, huh? There is no reason to preserve them--you don't want them.

This comment has been minimized.

Show comment Hide comment
@takluyver

takluyver May 9, 2011

Member

...In one go? Or as soon as you get to 4096 bytes, it crashes?

So does this bit of the code only get executed under pythonw.exe? I see the check for the executable file has gone, is one of the conditionals equivalent?

@takluyver

takluyver May 9, 2011

Member

...In one go? Or as soon as you get to 4096 bytes, it crashes?

So does this bit of the code only get executed under pythonw.exe? I see the check for the executable file has gone, is one of the conditionals equivalent?

This comment has been minimized.

Show comment Hide comment
@epatters

epatters May 10, 2011

Contributor

As soon as you hit 4096 bytes. See http://bugs.python.org/issue706263 for more information about the bug.

This bit of code is only executed under pythonw.exe and only when the streams are not explictly redirected by the kernel launch process. The latter condition is the reason that I can't leave the check for pythonw.exe in the kernel process itself: there is no way, as far as I can tell, for the kernel process to know if it has a '"bad" stdout or stderr. I am using the --no-stdout and --no-stderr options to communicate that information.

@epatters

epatters May 10, 2011

Contributor

As soon as you hit 4096 bytes. See http://bugs.python.org/issue706263 for more information about the bug.

This bit of code is only executed under pythonw.exe and only when the streams are not explictly redirected by the kernel launch process. The latter condition is the reason that I can't leave the check for pythonw.exe in the kernel process itself: there is no way, as far as I can tell, for the kernel process to know if it has a '"bad" stdout or stderr. I am using the --no-stdout and --no-stderr options to communicate that information.

This comment has been minimized.

Show comment Hide comment
@epatters

epatters May 10, 2011

Contributor

To be clear: the reason that I was able to do the check for pythonw.exe in the kernel process before was that there was no option to redirect the kernel's stdout or stderr. I could do whatever I wanted with them.

@epatters

epatters May 10, 2011

Contributor

To be clear: the reason that I was able to do the check for pythonw.exe in the kernel process before was that there was no option to redirect the kernel's stdout or stderr. I could do whatever I wanted with them.

@ellisonbg

This comment has been minimized.

Show comment Hide comment
@ellisonbg

ellisonbg May 10, 2011

Member

Just to clarify the need for this. Isn't the stdout/stderr of the kernel redirected to the zmq channel? What type of redirect is this then?

Member

ellisonbg commented May 10, 2011

Just to clarify the need for this. Isn't the stdout/stderr of the kernel redirected to the zmq channel? What type of redirect is this then?

@epatters

This comment has been minimized.

Show comment Hide comment
@epatters

epatters May 10, 2011

Contributor

It is redirected to the ZMQ channel if everything is set up correctly.

The use case that I have for this is as follows: my application launches a kernel in a way that should be transparent to the user. However, the kernel may crash while launching if, say, an unavailable matplotlib backend has been selected. In this case, the application needs to inform the user about the crash in a way that is actually useful. To do this, the exception message produced by the kernel must be captured.

Contributor

epatters commented May 10, 2011

It is redirected to the ZMQ channel if everything is set up correctly.

The use case that I have for this is as follows: my application launches a kernel in a way that should be transparent to the user. However, the kernel may crash while launching if, say, an unavailable matplotlib backend has been selected. In this case, the application needs to inform the user about the crash in a way that is actually useful. To do this, the exception message produced by the kernel must be captured.

@ellisonbg

This comment has been minimized.

Show comment Hide comment
@ellisonbg

ellisonbg May 10, 2011

Member

Ahh, OK, that really helps. So this is to cover a edge case that itself is exceptional.

Member

ellisonbg commented May 10, 2011

Ahh, OK, that really helps. So this is to cover a edge case that itself is exceptional.

@epatters

This comment has been minimized.

Show comment Hide comment
@epatters

epatters May 10, 2011

Contributor

Right. Is this ready to merge?

Contributor

epatters commented May 10, 2011

Right. Is this ready to merge?

@ellisonbg

This comment has been minimized.

Show comment Hide comment
@ellisonbg

ellisonbg May 10, 2011

Member

I am fine with it being merged. I didn't look too closely at the
code, but you know this side of things better than the rest of us. I
would go ahead and merge unless anyone else has comments.

On Tue, May 10, 2011 at 9:35 AM, epatters
reply@reply.github.com
wrote:

Right. Is this ready to merge?

Reply to this email directly or view it on GitHub:
#420 (comment)

Brian E. Granger
Cal Poly State University, San Luis Obispo
bgranger@calpoly.edu and ellisonbg@gmail.com

Member

ellisonbg commented May 10, 2011

I am fine with it being merged. I didn't look too closely at the
code, but you know this side of things better than the rest of us. I
would go ahead and merge unless anyone else has comments.

On Tue, May 10, 2011 at 9:35 AM, epatters
reply@reply.github.com
wrote:

Right. Is this ready to merge?

Reply to this email directly or view it on GitHub:
#420 (comment)

Brian E. Granger
Cal Poly State University, San Luis Obispo
bgranger@calpoly.edu and ellisonbg@gmail.com

epatters added a commit that referenced this pull request May 11, 2011

Merge pull request #420 from epatters/kernel-stream-redirect
Permit kernel std* to be redirected

@epatters epatters merged commit 8879791 into ipython:master May 11, 2011

mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014

Merge pull request #420 from epatters/kernel-stream-redirect
Permit kernel std* to be redirected
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment