ios: finish bridging between ObjC/Swift #312
Conversation
Signed-off-by: Michael Rebello <me@michaelrebello.com>
|
This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
|
Waiting on #316 |
Signed-off-by: Michael Rebello <me@michaelrebello.com>
|
This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
Signed-off-by: Michael Rebello <me@michaelrebello.com>
Signed-off-by: Michael Rebello <me@michaelrebello.com>
Signed-off-by: Michael Rebello <me@michaelrebello.com>
Signed-off-by: Michael Rebello <me@michaelrebello.com>
Signed-off-by: Michael Rebello <me@michaelrebello.com>
Signed-off-by: Michael Rebello <me@michaelrebello.com>
Signed-off-by: Michael Rebello <me@michaelrebello.com>
Signed-off-by: Michael Rebello <me@michaelrebello.com>
Signed-off-by: Michael Rebello <me@michaelrebello.com>
Signed-off-by: Michael Rebello <me@michaelrebello.com>
Signed-off-by: Michael Rebello <me@michaelrebello.com>
rebello95
changed the title
Signed-off-by: Michael Rebello <me@michaelrebello.com>
| return EnvoyStreamEmitter(stream: httpStream) | ||
| } | ||
|
|
||
| public func sendUnary(_ request: Request, body: Data?, |
There was a problem hiding this comment.
Instead of startStream and sendUnary, I'd suggest:
send(request:handler:) and send(request:body:handler:)
There was a problem hiding this comment.
or sendRequest(_:handler:) / sendRequest(_:body:handler:)
There was a problem hiding this comment.
I'd rather lean towards being clear on the intent (stream/unary) for the end consumer's sake, rather than making them assume based on function parameter signatures
There was a problem hiding this comment.
And of course trailers can be in there too, but since trailers are so rare in normal cases, I'd probably still suggest a signature which excludes or defaults them.
There was a problem hiding this comment.
I understand the argument about clarity/specificity, but I doubt most end-users are likely to be thinking in these terms, and I have misgivings because I don't really think this actually is more accurate.
The startStream interface is a perfectly appropriate interface for sending a "unary" request, and the current naming sort of makes it sound like you should only use it when you're trying to stream something (a naive user might even think that this is to stream a response, which is completely unrelated).
In fact, all HTTP requests are "unary" in the sense that they have a single request and response. I feel like these are semi-GRPC concepts that are leaking in because GRPC layers a stream encoding into a "unary" HTTP request.
There was a problem hiding this comment.
I guess in general, since this is a top level interface, I'm pushing back a little because I feel the naming should be recognizable and understandable to users who don't have a deep familiarity with the underlying concerns.
There was a problem hiding this comment.
Talked offline - this makes sense. Going to update in a separate PR after merging this
**This is a working end-to-end iOS implementation using Envoy Mobile as a library!** Depends on #312 and #353. Updates our Swift example app to use the Envoy Mobile direct interfaces rather than using `URLSession` and Envoy as a listener. Note: For now, we are no longer showing the text response of the response body due to the fact that the underlying implementation is not yet complete for bodied requests (headers-only). Signed-off-by: Michael Rebello <me@michaelrebello.com> 
Per the discussion [here](#312 (comment)), updating the unary and streaming interfaces. Also moving the non-streaming convenience function into an extension on the protocol type so that it's available to all consumers. Lastly, added a test to validate the default behavior of this extension function. Signed-off-by: Michael Rebello <me@michaelrebello.com>
Per the discussion [here](#312 (comment)), updating the unary and streaming interfaces. - Moved the non-streaming convenience function into an extension on the protocol type so that it's available to all consumers - Added a test to validate the default behavior of this extension function - Added a `CancelableStream` protocol which includes a subset of functionality from the `StreamEmitter`, allowing consumers of the unary function to cancel requests without having the ability to send additional data into the stream Signed-off-by: Michael Rebello <me@michaelrebello.com>
**This is a working end-to-end iOS implementation using Envoy Mobile as a library!** Depends on envoyproxy/envoy-mobile#312 and envoyproxy/envoy-mobile#353. Updates our Swift example app to use the Envoy Mobile direct interfaces rather than using `URLSession` and Envoy as a listener. Note: For now, we are no longer showing the text response of the response body due to the fact that the underlying implementation is not yet complete for bodied requests (headers-only). Signed-off-by: Michael Rebello <me@michaelrebello.com>  Signed-off-by: JP Simard <jp@jpsim.com>
Per the discussion [here](envoyproxy/envoy-mobile#312 (comment)), updating the unary and streaming interfaces. - Moved the non-streaming convenience function into an extension on the protocol type so that it's available to all consumers - Added a test to validate the default behavior of this extension function - Added a `CancelableStream` protocol which includes a subset of functionality from the `StreamEmitter`, allowing consumers of the unary function to cancel requests without having the ability to send additional data into the stream Signed-off-by: Michael Rebello <me@michaelrebello.com> Signed-off-by: JP Simard <jp@jpsim.com>
**This is a working end-to-end iOS implementation using Envoy Mobile as a library!** Depends on envoyproxy/envoy-mobile#312 and envoyproxy/envoy-mobile#353. Updates our Swift example app to use the Envoy Mobile direct interfaces rather than using `URLSession` and Envoy as a listener. Note: For now, we are no longer showing the text response of the response body due to the fact that the underlying implementation is not yet complete for bodied requests (headers-only). Signed-off-by: Michael Rebello <me@michaelrebello.com>  Signed-off-by: JP Simard <jp@jpsim.com>
Per the discussion [here](envoyproxy/envoy-mobile#312 (comment)), updating the unary and streaming interfaces. - Moved the non-streaming convenience function into an extension on the protocol type so that it's available to all consumers - Added a test to validate the default behavior of this extension function - Added a `CancelableStream` protocol which includes a subset of functionality from the `StreamEmitter`, allowing consumers of the unary function to cancel requests without having the ability to send additional data into the stream Signed-off-by: Michael Rebello <me@michaelrebello.com> Signed-off-by: JP Simard <jp@jpsim.com>
This PR ties together the upstream Objective-C/++ implementation with the Swift interfaces/bridging implementation. After this change, end-to-end calls from the native Swift library into the Envoy core should be supported.
EnvoyStreamEmitterwhich sends requests from Swift down into Objective-C[String: [String]]versus[String: String])ResponseHandleras a class which provides a pass-through for setting Objective-C blocks directly from Swift for handling responses#349 will use this interface with the Swift demo apps.
Signed-off-by: Michael Rebello me@michaelrebello.com