Allows buildpack output to stream commands #787
Conversation
| @@ -35,6 +35,8 @@ use crate::buildpack_output::util::{prefix_first_rest_lines, prefix_lines, Parag | |||
| use crate::write::line_mapped; | |||
There was a problem hiding this comment.
I'm ready for feedback on if this is a viable path forward or what else might be needed.
Exploring our options for solving this problem
- Give the end user a struct like
LockedWriterthat allows them to clone and manage their own IO faux duplication- Technically possible and quite explicit, but is gross for the buildpack maintainer. I feel we should take on this complexity, not our end users.
- Introduce a Command output function variant that only takes a single writer
- I tried this, the function would then have to return the writer and any Result object, likely via a tuple, which is ergonomically weird.
- Introduce a non-consuming function that yields two
Writestructs that are then proxied to the actual Write output (This PR).- This PR: Solves the problem. The downside is that it's complex, but it's hidden from the user. There may be an easier way to implement this logic. I feel this closure type of build output streaming is the correct interface as it allows natively interfacing with the result and not having to juggle returning a build output struct.
There was a problem hiding this comment.
fwiw, i've been pretty happy with the closure variant for streaming command outputs.
There was a problem hiding this comment.
@schneems I noticed you added the "Skip Changelog" label here. Why no changelog entry?
Good spot. I suspect it's because when this PR was opened, the I've removed the skip changelog label now. |
schneems
force-pushed
the
schneems/output-multi-stream
branch
from
278fec7 to
47b51b4
Compare
|
Integration tests are failing. They're also failing for me on main locally #802 |
|
Ah the I've retriggered the CI job now. |
|
Thankfully a restart fixed the issue. Changelog is added. Tests are passing. |
|
I'm going to rebase this and then I want to merge it in. If there's a refactor needed or some changes needed, then I'll own it. If it's not up to standards or quality, then we need to figure out additional ways to scale out standard and quality measures. |
schneems
force-pushed
the
schneems/output-multi-stream
branch
from
47b51b4 to
cf721cc
Compare
|
Think we can close in favor of using https://crates.io/crates/bullet_stream |
The
start_streaminterface only returns a single writer. The Command expects two writers: stdout and stderr. There's currently no good way for a buildpack author to stream a running Command, to the point that there are no examples of it in the documentation.This PR introduces a mechanism to do this and adds a doc that executes a command streaming to the build output.