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

fix: stream container build output and cap max buffer size #602

Merged
merged 1 commit into from
Mar 8, 2019

Conversation

eysi09
Copy link
Collaborator

@eysi09 eysi09 commented Mar 7, 2019

This is a quick fix for #399 (addresses point one in this comment).

A few things:

  • I opted not to append the stderr to the output string in the spawn function since the error is already printed at the top of the error message. This also conforms to how the container build output was displayed before this change.
  • We now only print the last hundred lines of the output in case of an error. Should this be more?
  • The MAX_BUFFER_SIZE is pretty arbitrary, any thoughts?

@eysi09 eysi09 force-pushed the stream-container-build-logs branch from 28aa279 to 2a885c8 Compare March 7, 2019 16:11
Copy link
Collaborator

@thsig thsig 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 to me. ~1MB sounds like an OK default max buffer size for starters too.

Any further thoughts on this one, @edvald?

@edvald edvald merged commit a94de26 into master Mar 8, 2019
@edvald edvald deleted the stream-container-build-logs branch March 8, 2019 10:34
@edvald
Copy link
Collaborator

edvald commented Mar 8, 2019

Nope, looks good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants