Add target and generated code for QPS #1827
Conversation
Motivation: The first step in implementing QPS testing for V2 is to add the proto files representing the services and messages used in QPS and generating the code for them. Modifications: - Modified the fetch script to add the proto files we need - Modified the generate script to generate code for them -Created executable target for QPS worker Result: We can start implementing the performance worker.
Protos/fetch.sh
Outdated
|
|
||
| # Clone the grpc and google protos into the staging area. | ||
| git clone --depth 1 https://github.com/grpc/grpc-proto "$checkouts/grpc-proto" | ||
| git clone --depth 2 https://github.com/grpc/grpc-proto "$checkouts/grpc-proto" |
There was a problem hiding this comment.
Why did the depth change to 2?
There was a problem hiding this comment.
Thanks for pointing this out, I left it by mistake in the PR. (it's from something I tried but wasn't necessary)
| mkdir -p "$upstream/grpc" | ||
| mkdir -p "$upstream/google" | ||
| mkdir -p "$upstream/grpc/testing" | ||
| mkdir -p "$upstream/grpc/core" |
There was a problem hiding this comment.
nit, we no longer need to mkdir grpc if we make subdirectories within it:
| mkdir -p "$upstream/grpc" | |
| mkdir -p "$upstream/google" | |
| mkdir -p "$upstream/grpc/testing" | |
| mkdir -p "$upstream/grpc/core" | |
| mkdir -p "$upstream/google" | |
| mkdir -p "$upstream/grpc/testing" | |
| mkdir -p "$upstream/grpc/core" |
Protos/generate.sh
Outdated
| done | ||
| } | ||
|
|
||
| function generate_qps_code { |
There was a problem hiding this comment.
can we call this generate_worker_service? The previous naming we used was qps, but I'm not sure where that actually came from.
|
|
||
| local output="$root/Sources/performance-worker/Generated" | ||
|
|
||
| generate_message "$here/upstream/grpc/core/stats.proto" "$here/upstream" "$output" "Visibility=Internal" "FileNaming=PathToUnderscores" |
There was a problem hiding this comment.
here we use "FileNaming=PathToUnderscores" but below we use "FileNaming=DropPath" – why?
There was a problem hiding this comment.
There are 2 stat.proto file - one in grpc/core and one in grpc/testing. As all the other .proto files are located within 'grpc/testing' I was thinking we can drop the path from their names, and keep the path only for the 'stats.proto' file from the other module, to differentiate the 2 files containing generated code for the 2 'stats.proto' files.
There was a problem hiding this comment.
Oh, I didn't realise there were two, thanks for explaining. In that case I'd rather we stick to one naming style if possible.
Protos/generate.sh
Outdated
|
|
||
| for proto in "${protos[@]}"; do | ||
| generate_message "$proto" "$here/upstream" "$output" "Visibility=Internal" "FileNaming=DropPath" | ||
| generate_grpc "$proto" "$here/upstream/" "$output" "Visibility=Internal" "Server=true" "_V2=true" "FileNaming=DropPath" |
There was a problem hiding this comment.
| generate_grpc "$proto" "$here/upstream/" "$output" "Visibility=Internal" "Server=true" "_V2=true" "FileNaming=DropPath" | |
| generate_grpc "$proto" "$here/upstream" "$output" "Visibility=Internal" "Server=true" "_V2=true" "FileNaming=DropPath" |
| ) | ||
|
|
||
| local output="$root/Sources/performance-worker/Generated" |
There was a problem hiding this comment.
| ) | |
| local output="$root/Sources/performance-worker/Generated" | |
| ) | |
| local output="$root/Sources/performance-worker/Generated" |
Protos/generate.sh
Outdated
|
|
||
| for proto in "${protos[@]}"; do | ||
| generate_message "$proto" "$here/upstream" "$output" "Visibility=Internal" "FileNaming=PathToUnderscores" | ||
| generate_grpc "$proto" "$here/upstream" "$output" "Visibility=Internal" "Server=true" "_V2=true" "FileNaming=PathToUnderscores" |
There was a problem hiding this comment.
We're actually generating client and server here even though we only set Server=true (true is the default for both client and server).
I think we only need to generate the server for worker_service.proto but need both for benchmark_service.proto.
There was a problem hiding this comment.
(Sorry, I didn't pick up on this in the last review!)
gjcairo
added
semver/none
3 participants
Motivation:
The first step in implementing QPS testing for V2 is to add the proto files representing the services and messages used in QPS and generating the code for them.
Modifications:
Result:
We can start implementing the performance worker.