rust: add Tonic version of generated Rust code#39
Merged
Conversation
Tonic (https://crates.io/crates/tonic) is, essentially, the Tokio 0.2 world's version of `tower-grpc`. Although the two libraries are similar and share a common geneology, the code generated by `tonic-build` is not compatible with the code generated by `tower-grpc-build`. In order to implement the mock destination controller for linkerd/linkerd2#4158 using the Tokio 0.2 stack, we need a Tonic version of the generated Rust bindings. This branch adds a new `linkerd2-proxy-api-tonic` crate that contains Tonic bindings generated from the proxy-api protos. Rather than putting this code in the existing `linkerd2-proxy-api` crate under a feature flag, I opted to create a separate crate. This is because `tonic-build` and `tower-grpc-build` will generate code that depends on different versions of libraries like `tower`, `http`, and `prost`. Although it would be possible to rename those dependencies so we can depend on both their Tokio 0.1 and Tokio 0.2 versions, it seems significantly more difficult to convince the code generators to generate code that references those dependencies by names like `http_02` and so on. Therefore, I created a separate crate for the Tonic version. This may also be useful when we start updating the proxy itself to use the Tokio 0.2 stack. Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
olix0r
reviewed
Apr 1, 2020
Member
olix0r
left a comment
olix0r
left a comment
There was a problem hiding this comment.
Given that this copy/pastes a bunch of code... do we want to hold off on merging this until we switch the proxy over to tonic? What are the blockers to doing that? (presumably, porting our gRPC interfaces after the 0.2 upgrade? anything else?)
Member
|
@hawkw there's no urgency on this, but what do you think about taking a branch-based approach for tokio-0.2 (as we did in the proxy repo)? |
Contributor
Author
|
@olix0r sure, I'd be fine to change to that approach here. |
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
olix0r
reviewed
May 19, 2020
Cargo.toml
Outdated
| members = [ | ||
| "rs" | ||
| "rs", | ||
| "rs-tonic", |
Member
There was a problem hiding this comment.
fwiw, I'm in favor of just replacing the rs crate
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
hawkw
added a commit
to linkerd/linkerd2-proxy
that referenced
this pull request
May 20, 2020
This branch updates the `linkerd2-proxy-api-resolve` crate to use `std::future` and Tonic. Getting this to work required some upstream changes to linkerd/linkerd2-proxy-api#39 to pass different feature flags to Tonic, but the change here is pretty straigntforward. Depends on #518 and will be rebased onto `master-tokio-02` when that branch merges. Signed-off-by: Eliza Weisman <[email protected]>
turns out reading the generated sources without this is *really bad* Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
it's nice, but the proxy docker build shouldn't depend on rustfmt Signed-off-by: Eliza Weisman <[email protected]>
olix0r
approved these changes
Jun 16, 2020
Co-authored-by: Oliver Gould <[email protected]>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Tonic (https://crates.io/crates/tonic) is, essentially, the Tokio 0.2
world's version of
tower-grpc. Although the two libraries are similarand share a common geneology, the code generated by
tonic-buildis notcompatible with the code generated by
tower-grpc-build. In order toimplement the mock destination controller for linkerd/linkerd2#4158
using the Tokio 0.2 stack, we need a Tonic version of the generated Rust
bindings.
This branch adds a new
linkerd2-proxy-api-toniccrate that containsTonic bindings generated from the proxy-api protos. Rather than putting
this code in the existing
linkerd2-proxy-apicrate under a featureflag, I opted to create a separate crate. This is because
tonic-buildand
tower-grpc-buildwill generate code that depends on differentversions of libraries like
tower,http, andprost. Although itwould be possible to rename those dependencies so we can depend on both
their Tokio 0.1 and Tokio 0.2 versions, it seems significantly more
difficult to convince the code generators to generate code that
references those dependencies by names like
http_02and so on.Therefore, I created a separate crate for the Tonic version.
This may also be useful when we start updating the proxy itself to use
the Tokio 0.2 stack.
Signed-off-by: Eliza Weisman [email protected]