Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate stdout/stderr streams for one-off commands #1101

Merged
merged 3 commits into from
Aug 10, 2017
Merged

Separate stdout/stderr streams for one-off commands #1101

merged 3 commits into from
Aug 10, 2017

Conversation

ejholmes
Copy link
Contributor

Closes #1044

This separates stdout from stderr in emp run, so that emp run can be used with existing UNIX tools more easily.

Implementation wise, the endpoint for running one-offs now multiplexes stdout/stderr over the same TCP connection, and the client de-multiplexes it before writing it to the terminal's stdout/stderr. The multiplex/de-multiplex is done using the stdcopy, which is pulled from Docker's internals.

This is 100% backwards compatible. Older clients will receive the combined stream, while newer clients (those that send X-Multiplex: 1 in the headers), will get a multiplexed stream. And inversely, new clients talking with an older daemon will continue to handle the combined stream.

This refactors the internal `empire.Empire.Run` method to support
separate I/O streams for stdout and stderr. Previously, stdout and
stderr from the attached container would be merged together.

This comment does not change the Heroku API to include support for
multiplexing the streams, so stdout/stderr will still be merged from the
clients perspective.
Copy link
Contributor

@phobologic phobologic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! This will be awesome.

@ejholmes ejholmes changed the base branch from stdcopy to master August 10, 2017 22:42
@ejholmes ejholmes merged commit 9bb9e0c into master Aug 10, 2017
@ejholmes ejholmes deleted the unix branch August 10, 2017 22:52
@ejholmes ejholmes mentioned this pull request Sep 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants