ReadStream implementation #130
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Monkatraz
approved these changes
May 20, 2024
32 tasks
masad-frost
added a commit
that referenced
this pull request
May 21, 2024
* ReadStream implementation * remove tee for now * Make the stream itself an iterable * Implement return for iterator * comment
masad-frost
added a commit
that referenced
this pull request
May 21, 2024
* ReadStream implementation * remove tee for now * Make the stream itself an iterable * Implement return for iterator * comment
masad-frost
added a commit
that referenced
this pull request
May 29, 2024
* ReadStream implementation * remove tee for now * Make the stream itself an iterable * Implement return for iterator * comment
masad-frost
added a commit
that referenced
this pull request
May 31, 2024
* ReadStream implementation * remove tee for now * Make the stream itself an iterable * Implement return for iterator * comment
masad-frost
added a commit
that referenced
this pull request
Jun 6, 2024
* ReadStream implementation * remove tee for now * Make the stream itself an iterable * Implement return for iterator * comment
masad-frost
added a commit
that referenced
this pull request
Jun 11, 2024
* ReadStream implementation * remove tee for now * Make the stream itself an iterable * Implement return for iterator * comment
masad-frost
added a commit
that referenced
this pull request
Jun 13, 2024
* ReadStream implementation * remove tee for now * Make the stream itself an iterable * Implement return for iterator * comment
masad-frost
added a commit
that referenced
this pull request
Jun 20, 2024
* ReadStream implementation * remove tee for now * Make the stream itself an iterable * Implement return for iterator * comment
masad-frost
added a commit
that referenced
this pull request
Jun 24, 2024
* ReadStream implementation * remove tee for now * Make the stream itself an iterable * Implement return for iterator * comment
masad-frost
added a commit
that referenced
this pull request
Aug 16, 2024
All the changes are documented in `Protocol.md` but here's a summary: - Handle invalid client requests by sending a close with an error back - This was the main motivation for the change. While we could sort-of implement this error response without the other changes, things are setup in such a way where it is very hard to implement correctly without deeper changes in how we handle closing. - Add more robust closing mechanics - Half-close states - Close signals from read end of the pipes - Abort full-closure (for errors and cancellation) - Switch from `Pushable` and `AsyncIterator` APIs to a `ReadStream` and `WriteStream` - All procedures have `init` and some have `input` While the changes are not strictly backwards compatible, hence the major protocol bump, the system can still operate across versions to some extent. See PRs linked below for more information on the above # TODOs - [x] Define protocol and update doc #111 - [x] Design stream abstractions #118 - [x] Redsigned in #249 - [x] Implement stream abstractions - [x] ReadStream #130 - [x] WriteStream #132 - [x] All streams have init, some have input. - [x] Protocol change documented in #153 - [x] Implementation change #159 - [x] Use stream abstractions & implement protocol closing semantics - [x] Protocol: Implement close requests from readers #165 - [x] Protocol: Implement half-close - [x] Client #162 - [x] Server #163 - [x] Simple s/Pushable/Stream replacement - [x] Client #136 - [x] Server #137 - [x] Make `Input` iterator on the server use `Result` so we can signal stream closes, client disconnects, and aborts #172 - [x] Add Abort mechanism - [x] Docs update #175 - [x] Implement abort - [x] Client #193 - [x] Server #200 - [x] Add `INVALID_REQUEST` to schema #107 - [x] Handle/send back `INVALID_REQUEST` errors with an abort bit #203 - [x] Handle/send back `INTERNAL_RIVER_ERROR` with an abort bit #203 - [x] Send abort bit with `UNCAUGHT_ERROR` #201 - [x] Abort tombstones #204 - [ ] Try to find uncovered areas to test - [ ] `undefined` value for `init`, `input`, & `output`. - [ ] Update docs - [ ] Changelog --------- Co-authored-by: Jacky Zhao <j.zhao2k19@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why
Implementation for read stream from #111
and #118
What changed
Reduced the API surface area since the RFC removing "break" stuff,
drainis now how you can interrupt iteration. Otherwise, just implementation and tests for ReadStream.Removed
teefrom the interface for now.