Closed
Conversation
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
olix0r
reviewed
Apr 17, 2020
Member
olix0r
left a comment
There was a problem hiding this comment.
I really like the with simplification. a few questoins
olix0r
reviewed
Apr 17, 2020
Signed-off-by: Eliza Weisman <[email protected]>
hawkw
added a commit
that referenced
this pull request
Apr 22, 2020
This branch updates the core proxy components to use `std::future` and `tower` 0.3. This includes most of the `app` crate and its components, the `transport` crate, and some miscellaneous stuff in `proxy` that was necessary for everything else to work. I know this is a huge PR, but unfortunately, the coupling between these crates made it kind of necessary. In theory, the proxy's constituent crates are fairly loosely coupled, and in *most* cases, we don't need to make this kind of cross-cutting change, but using Tower traits as an interface is a big part of *how* we are able to implement this loose coupling. Given the constraint that the `master-tokio-0.2` branch should always compile, I don't think it's possible to do the `std::future` update without a branch like this. Fortunately, although large, the change here is mostly pretty mechanical. There's a lot of udpdating `Service` implementations to the new Tower 0.3 version of the trait, including some slight tweaks to future implementations to work with stack pinning. Some code (such as tap and the controller client) had to be commented out to avoid having to update additional proxy subsystems end-to-end in this branch; we will update those components in follow-up PRs. I haven't updated from tokio 0.1 async io to tokio 0.2 async IO; that will be a later pass. This is because having both `hyper` 0.12 and `tokio` 0.2 with the "net" feature in the dependency graph at the same time pulls in incompatible `iovec` versions and is _very_ hard to untangle. `hyper` and the io types (the `AsyncRead`/`Write` traits, sockets, etc) will have to move together. Once this PR lands and the new Tower version is wired all the way through the core of the proxy, it should be pretty easy to update the rest of the crates in separate, independent PRs. We can essentially go through the stack layer-by-layer, and update each component. Once that's done, then we can circle back and update Hyper and the IO traits to the Tokio 0.2 versions. Closes #474 (it's contained in this branch and can't be merged independently without breaking stuff) Signed-off-by: Eliza Weisman <[email protected]>
Contributor
|
@hawkw This can be closed now correct? |
Contributor
Author
|
@kleimkuhler yup, I thought the |
Contributor
|
I think that may just be for closing issues |
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
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.
This updates the
draincrate to use std::future.Note that because Tokio 0.2's
spawnreturns aJoinHandlethat maybe used to await a task's completion, we can probably simplify the
"waiting for tasks to drain" logic a bit. I didn't do that because I wanted
to do as close to a strict translation as possible, without adding possible
changes in behavior. Also, we are currently still using the compat runtime's
0.1-style spawning.