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
L92: grpc-dotnet gRPC-Web proposal #169
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for writing this!
L63-dotnet-grpc-web.md
Outdated
var client = Greeter.GreeterClient(channel); | ||
``` | ||
|
||
### Interoperability |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stanley-cheung looks like grpc-web doesn't have a formal spec for a standard set of "grpc-web interop tests" (a similar document to what we have for regular interop tests https://github.com/grpc/grpc/blob/master/doc/interop-test-descriptions.md). I think such doc will become increasingly important as more grpc-web implementations are added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw, if we don't have a better spec in mind, some regular interop tests could be repurposed to be used for grpc-web as well. There might be other grpc-web specific test cases in addition to that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Repurposing the regular interop tests is something to consider. The client and duplex streaming calls can be ignored, but there is still a lot of tests.
What is the next step with this proposal? |
L63-dotnet-grpc-web.md
Outdated
|
||
gRPC requires HTTP/2 features that are not available in browser HTTP clients. [gRPC-Web](https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-WEB.md) modifies the gRPC specification to allow gRPC to be callable from the browser. | ||
|
||
Traditionally gRPC-Web support is added to a service via a proxy. Browsers call the proxy and the proxy translates the gRPC-Web call to gRPC HTTP/2 and forwards it on to the service implentation. This works, but there is effort, complexity and performance costs associated with adding a proxy to a solution. An in-process proxy allows for gRPC-Web to be supported by adding additional packages and startup configuration to an existing app. Being able to support gRPC-Web with an in-process proxy removes many of the costs of the traditional gRPC-Web proxy. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Glad to see finally gRPC-Web is integrated with gRPC 👍
L63-dotnet-grpc-web.md
Outdated
|
||
The grpc-dotnet gRPC server is built on ASP.NET Core and hosted on Kestrel, a full-featured, multi-purpose HTTP/1.1 and HTTP/2 server. ASP.NET Core has a [request pipeline that an application can plug middleware into](https://docs.microsoft.com/aspnet/core/fundamentals/middleware) to process the request and response. ASP.NET Core also supports configuring CORS for an application. | ||
|
||
The grpc-dotnet gRPC client is built on HttpClient, a full-featured, multi-purpose HTTP/1.1 and HTTP/2 client. HttpClient has an [outgoing request pipe that an application can plug handlers into](https://docs.microsoft.com/dotnet/api/system.net.http.httpmessagehandler). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can HTTP/1.1 and HTTP/2 being ran on the same connection? Or the new implementation needs to manage two types of subchannel
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nope, you either have HTTP/1.1 connection or you have HTTP/2.
From users perspective, you either configure a Channel to create HTTP1.1 connections and use grpc-web or you configure it to use regular grpc wire protocol over HTTP2.
@stanley-cheung @wenbozhu we need to choose the reviewers for this PR. @stanley-cheung @wenbozhu we need to finalize the grpc-web interop test spec (internal b/147574732) and list it as one of the requirements to reach GA. We also need to formulate the other criteria for making the .NET grpc-web implementation. We are not really in a hurry with these as we want to allow some time for users to experiment with the preview version grpc .NET grpc-web and report possible issues / usability friction. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a few comments and I'll approve and merge once they are resolved.
be41c77
to
f8b8c91
Compare
Feedback applied. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Proposal for adding gRPC-Web support to grpc-dotnet using an in-process proxy and a gRPC-Web client.
@jtattermusch
Preview URL: https://github.com/JamesNK/proposal/blob/jamesnk/dotnet-grpc-web/L92-dotnet-grpc-web.md