Allow to write a sequence of requests/responses #1499
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
FranzBusch
commented
Oct 7, 2022
Comment on lines
78
to
86
| @inlinable | ||
| public func send<S: Sequence>( | ||
| contentsOf requests: S | ||
| ) async throws where S.Element == (Request, Compression) { | ||
| try await self.asyncWriter.yield(contentsOf: requests) | ||
| } |
There was a problem hiding this comment.
@Lukasa @fabianfett What do you think about using the tuple here? Do you prefer creating a new type like MessageWithCompression?
There was a problem hiding this comment.
I'm almost always in opposition to tuples: nominal types FTW.
There was a problem hiding this comment.
Fine either way. But generally structs are the better choice for evolving types and APIs. Name should probably be MessageAndCompression? With sounds like the message was already compressed.
There was a problem hiding this comment.
Actually I changed the API a bit for now where you can write a sequence of requests/responses and set the compression for all of them. If users ever want to write a sequence with different compression per request/response we can still add that API. WDYT?
FranzBusch
force-pushed
the
fb-write-sequence
branch
from
70c5afe
to
a56ff4c
Compare
# Motivation We recently adopted the `NIOAsyncWriter` to back the `GRPCAsyncRequestStreamWriter` and the `GRPCAsyncResponseStreamWriter`; however, we did not expose the functionality to write a sequence of the elements that the `NIOAsyncWriter` offers. This can be useful in cases where you want to write a batch of requests/responses since it reduces the amount of locks and thread hops. # Modification Expose new methods on the `GRPCAsyncRequestStreamWriter` and the `GRPCAsyncResponseStreamWriter` to enable to write a sequence. I also fixed up the comments for the `GRPCAsyncRequestStreamWriter` since they were outdated. # Result Users can write a batch of requests/responses.
FranzBusch
force-pushed
the
fb-write-sequence
branch
from
a56ff4c
to
2633ad2
Compare
Lukasa
approved these changes
Oct 7, 2022
Lukasa
requested changes
Oct 7, 2022
Lukasa
approved these changes
Oct 7, 2022
WendellXY
pushed a commit
to sundayfun/grpc-swift
that referenced
this pull request
Aug 24, 2023
* Allow to write a sequence of requests/responses # Motivation We recently adopted the `NIOAsyncWriter` to back the `GRPCAsyncRequestStreamWriter` and the `GRPCAsyncResponseStreamWriter`; however, we did not expose the functionality to write a sequence of the elements that the `NIOAsyncWriter` offers. This can be useful in cases where you want to write a batch of requests/responses since it reduces the amount of locks and thread hops. # Modification Expose new methods on the `GRPCAsyncRequestStreamWriter` and the `GRPCAsyncResponseStreamWriter` to enable to write a sequence. I also fixed up the comments for the `GRPCAsyncRequestStreamWriter` since they were outdated. # Result Users can write a batch of requests/responses. * Update tests
pinlin168
pushed a commit
to sundayfun/grpc-swift
that referenced
this pull request
Aug 24, 2023
* Allow to write a sequence of requests/responses # Motivation We recently adopted the `NIOAsyncWriter` to back the `GRPCAsyncRequestStreamWriter` and the `GRPCAsyncResponseStreamWriter`; however, we did not expose the functionality to write a sequence of the elements that the `NIOAsyncWriter` offers. This can be useful in cases where you want to write a batch of requests/responses since it reduces the amount of locks and thread hops. # Modification Expose new methods on the `GRPCAsyncRequestStreamWriter` and the `GRPCAsyncResponseStreamWriter` to enable to write a sequence. I also fixed up the comments for the `GRPCAsyncRequestStreamWriter` since they were outdated. # Result Users can write a batch of requests/responses. * Update tests
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.
Motivation
We recently adopted the
NIOAsyncWriterto back theGRPCAsyncRequestStreamWriterand theGRPCAsyncResponseStreamWriter; however, we did not expose the functionality to write a sequence of the elements that theNIOAsyncWriteroffers. This can be useful in cases where you want to write a batch of requests/responses since it reduces the amount of locks and thread hops.Modification
Expose new methods on the
GRPCAsyncRequestStreamWriterand theGRPCAsyncResponseStreamWriterto enable to write a sequence. I also fixed up the comments for theGRPCAsyncRequestStreamWritersince they were outdated.Result
Users can write a batch of requests/responses.