grpc client template with logging, metric and circuit-breaker #627
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rpatali
changed the title
initial gRPC implementation delegated everything directly to YARPC. key changes in this diff: - gRPC client template has logging, metric and circuit breaking - Proto file parsing for extracting methods and req/res types - implement SpecProvider interface for use in incremental build as well as template hydrate
although yarpc supports other protocols, the structs we have is designed for grpc support specifically so gRPC in the name is the right abstraction instead of YARPC.
rpatali
force-pushed
the
rpatali/yarpc-client-template
branch
from
7e49bdc to
a9b6b6c
Compare
jacobgreenleaf
suggested changes
Aug 27, 2019
codegen/templates/grpc_client.tmpl
Outdated
| RequestVolumeThreshold: requestVolumeThreshold, | ||
| Timeout: timeoutVal, | ||
| }) | ||
| return circuitBreakerDisabled |
There was a problem hiding this comment.
shouldn't this be return true?
jacobgreenleaf
approved these changes
Aug 27, 2019
| } | ||
| } | ||
|
|
||
| func configureCicruitBreaker(deps *module.Dependencies, timeoutVal int) bool { |
There was a problem hiding this comment.
I think we should create a common template the common parts of the HTTP, GRPC and Tchannel templates to avoid duplicate templates. There's some info here on nested templates: https://golang.org/pkg/text/template/
There was a problem hiding this comment.
I think we should create a common template the common parts of the HTTP, GRPC and Tchannel templates to avoid duplicate templates. There's some info here on nested templates: https://golang.org/pkg/text/template/
Good point. Makes sense.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I'm working on adding a few more tests but in the meanwhile, wanted to get an early review going. Some more refactor is in the progress as well.