Add grpc samples #65
Add grpc samples #65
Conversation
qu1queee
force-pushed
the
qu1queee/grpc
branch
2 times, most recently
from
c22aa20 to
adfab6d
Compare
There was a problem hiding this comment.
Hi @qu1queee , thank you for putting together this sample! Looking forward to get it in finally. I do have a few comments mainly related to the run script, which I would like to discuss with you.
Next, I suggest that we plan a pairing session.
Best regards, Rico
grpc/run.sh
Outdated
| @@ -0,0 +1,31 @@ | |||
| #!/bin/bash | |||
|
|
|||
| set -euo pipefail | |||
There was a problem hiding this comment.
In order to allow it being executed as part of test automation and to prevent users from having components being installed that incur costs, without the users understanding the full impact of them running the script, we'll need to add a cleanup routine to this script. See https://github.com/IBM/CodeEngine/blob/main/function2job/run for an example.
| fmt.Printf("using local endpoint: %s\n", localEndpoint) | ||
| conn, err := grpc.Dial(localEndpoint, grpc.WithTransportCredentials(insecure.NewCredentials())) | ||
| if err != nil { | ||
| log.Fatalf("failed to connect: %v", err) |
There was a problem hiding this comment.
Shouldn't we and stop the app initialization at this point?
There was a problem hiding this comment.
log.Fatalf comes with an os.Exit(1)
| Check the source code if you want to understand how this works. In general, | ||
| we provided three directories: | ||
|
|
||
| - `/ecommerce` directory hosts the protobuf files and declares the `grocery` |
There was a problem hiding this comment.
Is there every a need to regenerate this directory (e.g. after user made some adjustments to play around)? If so, I suggest to add the command to re-generate the files.
There was a problem hiding this comment.
done. Added a new script call `./regenerate-grpc-code.sh
grpc/run.sh
Outdated
|
|
||
| # Buy an item from electronics and pay with 2000 | ||
| echo "[INFO] Going to buy an iphone" | ||
| curl -q "${URL}"/buygrocery/electronics/iphone/2000.0 |
There was a problem hiding this comment.
I suggest to add a command that checks whether the buy operation actually succeeded. If it doesn't we should end the script with exit code non-zero
qu1queee
force-pushed
the
qu1queee/grpc
branch
2 times, most recently
from
5e6b323 to
5a0cc98
Compare
|
@reggeenr thanks for the review. I addressed all your comments. In addition, I added the following:
|
qu1queee
force-pushed
the
qu1queee/grpc
branch
from
5a0cc98 to
8477d6e
Compare
Introduce grpc directory, containing a microservice architecture consisting of an internal grpc server, and a public client/server app. A run.sh script is provided, to easily deploy this architecture in Code Engine, as well as a README, explaining the ideas behind. Signed-off-by: Mahesh Kumawat <[email protected]> Signed-off-by: encalada <[email protected]> Signed-off-by: Mahesh Kumawat <[email protected]>
Addressing all good feedback, to inline with other CE samples. Signed-off-by: encalada <[email protected]>
qu1queee
force-pushed
the
qu1queee/grpc
branch
2 times, most recently
from
5466c75 to
bd6d33c
Compare
qu1queee
force-pushed
the
qu1queee/grpc
branch
from
bd6d33c to
99de465
Compare
- Add architecture diagram to readme - Refactor proto interface, to inline fields - Add build script - Recompile proto files Signed-off-by: encalada <[email protected]>
qu1queee
force-pushed
the
qu1queee/grpc
branch
from
99de465 to
0333a3f
Compare
Introduce grpc directory, containing a microservice architecture consisting of an internal grpc server, and a public client/server app.
A run.sh script is provided, to easily deploy this architecture in Code Engine, as well as a README, explaining the ideas behind.