-
Notifications
You must be signed in to change notification settings - Fork 161
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
pipe redirection #325
Comments
To be clear, I'm not saying the above is elegant. Exploring if there's any simple solution. |
This is really good analysis but I'm not 100% sure I understand it all, so please be patient with me :). First, some quick comments on points that caught my eye:
We've wanted a way to cancel a pipe since at least 726b08d. Our solution is cancelable promises, but even before they land we could have a spec mechanism like CancelCurrentPipe(rs) that will eventually be exposed through
This is kind of interesting and I think could eventually be massaged into something more elegant. It reminds me a bit of #146. But I agree it is complex and a simpler version would be nice. Second a question to see if I understand the problematic scenario: id.writable.write("hello");
id.writable.write("world");
rs.pipeTo(id.writable);
id.readable.pipeTo(ws); Our goal here in an ideal world for the outcome would be:
Is that right? Did I miss anything? If so, this doesn't seem too hard to solve. I'm not sure how we would specify the exact protocol (including the issues of #307, of how to identify that a stream is identity and can be skipped---maybe that is your redirectTo idea!? I think it is now that I re-read.) But I think an algorithm of "wait for ultimate-non-identity-ready, then do special data transfer" is doable. I guess the heart of this concern is coming up with a protocol such that, in the special case where once there is nothing buffered in the identity transform and both the Given that with #321 It would be good to prototype this kind of thing as it has the flavor of something that needs actual testing to see if the edge cases work. |
I just want to say that the more I think about this in combination with #307, the more I think redirectTo is actually kind of nice. Before each write, check |
I started feeling that we've been too optimistic about use of specialized
pipeTo()
algorithm given an identify transform between the pair of rs/ws.In the discussion about the
Request
object, I've been saying that.write()
even before theRequest
gets attached to the final sink (Uploading: write to a writable stream, or pass in a readable stream? yutakahirano/fetch-with-streams#30 (comment))..pipeTo()
internally even with existence of such a buffer.The following are reduced example of what I said:
Example:
id
is writable even beforeid.readable.pipeTo()
happens.Example: In the following two examples,
rs.pipeTo(ws)
should happen eventually and internally after draining chunks queued inid
.Note that not whole of the
rs.pipeTo(ws)
algorithm should always happen. Closure ofrs
should result in making onlyid
closed ifid.readable.pipeTo(ws)
is changed toid.readable.pipeTo(ws, { preventClose: true })
.I'd call the optimized main data transfer part of
.pipeTo()
as.specialDataTransfer()
which excludes close/error signal handling.So, let's think of what we should do for the following example:
Here are two issues:
id
already has two chunks queued in it. Ifid.readable.pipeTo(ws)
algorithm is the one ofReadableStream.pipeTo()
as-is, we cannot drain all the data fromid
untilws
stop exerting back-pressure.rs.pipeTo(id.writable)
, we never be able to switch tors.specialDataTransfer(ws)
.I guess, the solution is:
id.readable.pipeTo(ws)
is called regardless of back-pressure ofws
..redirectTo
, on theWritableStream
for.pipeTo()
algorithm to redirect.specialDataTransfer()
tows
.ReadableStream.pipeTo()
is required to check.redirectTo
before everydest.write()
and if it's set, it must switch to users.specialDataTransfer(dest.redirectTo)
.Maybe this could be evolved to address #307, too.
The text was updated successfully, but these errors were encountered: