nest icon indicating copy to clipboard operation
nest copied to clipboard

sample(04-grpc): add independent gRPC Client / Server sample respectively

Open youngkiu opened this issue 1 year ago • 3 comments
trafficstars

PR Checklist

Please check if your PR fulfills the following requirements:

  • [x] The commit message follows our guidelines: https://github.com/nestjs/nest/blob/master/CONTRIBUTING.md
  • [ ] Tests for the changes have been added (for bug fixes / features)
  • [ ] Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • [ ] Bugfix
  • [ ] Feature
  • [ ] Code style update (formatting, local variables)
  • [ ] Refactoring (no functional changes, no api changes)
  • [ ] Build related changes
  • [ ] CI related changes
  • [x] Other... Please describe: Add another gRPC sample

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • [ ] Yes
  • [x] No

Other information

https://github.com/youngkiu/nest/blob/sample/34-grpc-client-server/sample/34-grpc-client-server/README.md

  • c-2 --> s-2
[1] Calling SayHello with name:""
[1] Received error 3 INVALID_ARGUMENT: request missing required field: name
[2] Calling SayHello with name:"youngkiu"
[2] Received response Hello youngkiu
[3] Calling SayHelloStreamReply with name:""
[3] Received expected error 3 INVALID_ARGUMENT: request missing required field: name
[3] Received status with code=INVALID_ARGUMENT details=request missing required field: name
[4] Calling SayHelloStreamReply with name:"youngkiu"
[4] Received response Hello youngkiu
[4] Received response Hello youngkiu
[4] Received response Hello youngkiu
[4] Received response Hello youngkiu
[4] Received response Hello youngkiu
[4] Received status with code=OK details=OK
  • c-2 --> s-3
[1] Calling SayHello with name:""
[1] Received error 3 INVALID_ARGUMENT: request missing required field: name
[2] Calling SayHello with name:"youngkiu"
[2] Received response Hello youngkiu
[3] Calling SayHelloStreamReply with name:""
[3] Received status with code=OK details=OK
[4] Calling SayHelloStreamReply with name:"youngkiu"
[4] Received response Hello youngkiu
[4] Received response Hello youngkiu
[4] Received response Hello youngkiu
[4] Received response Hello youngkiu
[4] Received response Hello youngkiu
[4] Received status with code=OK details=OK

Currently, NestJS Server produces different results. This will be resolved when https://github.com/nestjs/nest/pull/13195 is released.

youngkiu avatar Mar 16 '24 05:03 youngkiu

Pull Request Test Coverage Report for Build 95db5acc-385b-40ba-8c36-c49c7e046909

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 92.146%

Totals Coverage Status
Change from base Build fad5078e-9529-46ad-977e-491c6acb241f: 0.0%
Covered Lines: 6734
Relevant Lines: 7308

💛 - Coveralls

coveralls avatar Mar 16 '24 06:03 coveralls

I wonder if adding yet another sample to this repository doesn't create even more unnecessary overhead that we already face with updating their dependencies (maintenance burden).

Is there any way we could merge this example with the existing grpc one? If not, I'd recommend creating a separate repository instead

kamilmysliwiec avatar Mar 17 '24 18:03 kamilmysliwiec

  1. The original 04-grpc consists of 1, 2, and 3 as one program, so it may be a little confusing for developers who are new to samples.
04-grpc Client Server
HTTP 1
gRPC 2 3
  1. I wanted to introduce ts-proto to create 04-grpc/src/hero/interfaces more easily,
  2. Usually in gRPC, client / server share proto files, so I thought a sample including the case of multiple proto files could be beneficial.

34-grpc-client-server was merged into 04-grpc, but if you still think the management burden is too high, it's okay to close the PR.

youngkiu avatar Mar 18 '24 14:03 youngkiu