Skip to content

Adds sample gRPC application#1261

Merged
google-prow-robot merged 2 commits intoknative:masterfrom
pivotal-cf-experimental:sample-grpc-app
Jun 21, 2018
Merged

Adds sample gRPC application#1261
google-prow-robot merged 2 commits intoknative:masterfrom
pivotal-cf-experimental:sample-grpc-app

Conversation

@bsnchan
Copy link
Copy Markdown
Contributor

@bsnchan bsnchan commented Jun 18, 2018

Pre-req: #707, #813

Follow up from #987

Proposed Changes

  • N/A

Release Note

NONE

* this sample depends on knative/serving#1047 to be complete
@google-prow-robot google-prow-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jun 18, 2018
@bsnchan
Copy link
Copy Markdown
Contributor Author

bsnchan commented Jun 18, 2018

/assign @evankanderson

Copy link
Copy Markdown
Member

@evankanderson evankanderson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve

Two small comments.

Comment thread sample/grpc-ping/client/client.go Outdated
flag.Parse()

opts := []grpc.DialOption{
grpc.WithInsecure(),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make this a flag for the future world where we have TLS sorted out for clusters with a public ingress?


grpcServer := grpc.NewServer()
ping.RegisterPingServiceServer(grpcServer, pingServer)
grpcServer.Serve(lis)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does grpcServer end up supporting h2c out of the box?

Might be worth a comment either documenting that it does, or mentioning that this is something that may need to be validated once #1047 is done.

@google-prow-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bsnchan, evankanderson

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-prow-robot google-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 18, 2018
* Adds comment that the sample grpc server serves h2c by default
@evankanderson
Copy link
Copy Markdown
Member

/lgtm

@google-prow-robot google-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 20, 2018
@google-prow-robot google-prow-robot merged commit dbc2f0c into knative:master Jun 21, 2018
creydr pushed a commit to creydr/knative-serving that referenced this pull request Mar 25, 2025
Co-authored-by: serverless-qe <serverless-support@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants