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

youngkiu · 2024-03-16T05:58:07Z

PR Checklist

Please check if your PR fulfills the following requirements:

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
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
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 #13195 is released.

coveralls · 2024-03-16T06:05:26Z

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
Change from base Build fad5078e-9529-46ad-977e-491c6acb241f:	0.0%
Covered Lines:	6734
Relevant Lines:	7308

💛 - Coveralls

kamilmysliwiec · 2024-03-17T18:16:16Z

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

youngkiu · 2024-03-18T14:21:17Z

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`

I wanted to introduce ts-proto to create 04-grpc/src/hero/interfaces more easily,
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.

ssilve1989 · 2024-03-26T21:16:19Z

sample/04-grpc/server/src/helloworld/helloworld.controller.ts

+  sayHelloStreamReply(request: HelloRequest): Observable<HelloReply> {
+    const hero$ = new ReplaySubject<HelloReply>();
+
+    if (request.name === '') {
+      hero$.error(
+        new RpcException({
+          code: grpc.status.INVALID_ARGUMENT,
+          message: 'request missing required field: name',
+        }),
+      );
+
+      return hero$.asObservable();
+    }
+
+    for (let i = 0; i < this.REPLY_COUNT; i++) {
+      hero$.next({ message: 'Hello ' + request.name });
+    }
+    hero$.complete();
+
+    return hero$.asObservable();
+  }


Why is this using a ReplaySubject? This can be written in multiple other ways without the need of a ReplaySubject. The most concise of which would be

sayHelloStreamReply({ name }: HelloRequest): Observable<HelloReply> { if (!name) { throw new RpcException({ code: status.INVALID_ARGUMENT, message: 'request missing required field: name', }); } return of({ message: `Hello ${name}` }).pipe(repeat(this.REPLY_COUNT)); }

but even, so I would caution showing an example of streaming synchronous replies like this. When the list of emissions is sufficiently large this can block the event loop writing out all the messages not allowing anything else to occur. This often results in health check failures in environments like kubernetes. Ideally each emission is written out asynchronously which can be achieved with scheduled and an async scheduler.

Thank you for the code review and suggestions.
I hope the sample code will greatly help developers using nest & grpc.

kamilmysliwiec · 2024-11-25T13:47:27Z

sample/04-grpc/client/src/interface/hero.ts

+  findMany(request: Observable<HeroById>): Observable<Hero>;
+}
+
+export function HeroesServiceControllerMethods() {


Any reason why we need this function? Is it auto-generated?

kamilmysliwiec · 2024-11-25T13:47:30Z

sample/04-grpc/client/src/interface/helloworld.ts

+  sayHelloStreamReply(request: HelloRequest): Observable<HelloReply>;
+}
+
+export function GreeterControllerMethods() {


Any reason why we need this function? Is it auto-generated?

youngkiu changed the title ~~sample(34-grpc-client-server): add independent gRPC Client / Server programs respectively~~ sample(34-grpc-client-server): add independent gRPC Client / Server sample respectively Mar 16, 2024

youngkiu mentioned this pull request Mar 16, 2024

docs(microservices): Add link to another gRPC sample nestjs/docs.nestjs.com#2993

Closed

10 tasks

youngkiu force-pushed the sample/34-grpc-client-server branch from 7e9dc01 to 3ec5be0 Compare March 18, 2024 13:44

youngkiu changed the title ~~sample(34-grpc-client-server): add independent gRPC Client / Server sample respectively~~ sample(04-grpc): add independent gRPC Client / Server sample respectively Mar 18, 2024

youngkiu added 5 commits March 20, 2024 08:43

sample(34-grpc-client-server): Develop server

52bff2d

sample(34-grpc-client-server): Develop client

a89e686

sample(34-grpc-client-server): Write README.md

f99c6b3

sample(34-grpc-client-server): skip a (proto) folder

a033729

sample(04-grpc): Merge independent gRPC Client / Server example

00c4047

youngkiu force-pushed the sample/34-grpc-client-server branch from 3ec5be0 to 00c4047 Compare March 19, 2024 23:44

sample(04-grpc): Rebase master branch

df08290

ssilve1989 reviewed Mar 26, 2024

View reviewed changes

sample(04-grpc): Update codes reviewed by @ssilve1989

af8fe73

youngkiu requested a review from ssilve1989 April 15, 2024 12:19

kamilmysliwiec reviewed Nov 25, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

youngkiu commented Mar 16, 2024 •

edited

Loading

coveralls commented Mar 16, 2024 •

edited

Loading

kamilmysliwiec commented Mar 17, 2024

youngkiu commented Mar 18, 2024 •

edited

Loading

ssilve1989 Mar 26, 2024

youngkiu Mar 27, 2024

kamilmysliwiec Nov 25, 2024 •

edited

Loading

kamilmysliwiec Nov 25, 2024 •

edited

Loading

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

Are you sure you want to change the base?

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

Conversation

youngkiu commented Mar 16, 2024 • edited Loading

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

coveralls commented Mar 16, 2024 • edited Loading

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

Details

💛 - Coveralls

kamilmysliwiec commented Mar 17, 2024

youngkiu commented Mar 18, 2024 • edited Loading

ssilve1989 Mar 26, 2024

Choose a reason for hiding this comment

youngkiu Mar 27, 2024

Choose a reason for hiding this comment

kamilmysliwiec Nov 25, 2024 • edited Loading

Choose a reason for hiding this comment

kamilmysliwiec Nov 25, 2024 • edited Loading

Choose a reason for hiding this comment

youngkiu commented Mar 16, 2024 •

edited

Loading

coveralls commented Mar 16, 2024 •

edited

Loading

youngkiu commented Mar 18, 2024 •

edited

Loading

kamilmysliwiec Nov 25, 2024 •

edited

Loading

kamilmysliwiec Nov 25, 2024 •

edited

Loading