This repository has been archived by the owner on Nov 15, 2023. It is now read-only.
Switch consensus crates to new futures #3146
Merged
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
tomaka
commented
Jul 19, 2019
| @@ -75,7 +75,7 @@ pub trait Proposer<B: BlockT> { | |||
| /// Error type which can occur when proposing or evaluating. | |||
| type Error: From<Error> + ::std::fmt::Debug + 'static; | |||
| /// Future that resolves to a committed proposal. | |||
| type Create: IntoFuture<Item=B, Error=Self::Error>; | |||
| type Create: Future<Output = Result<B, Self::Error>>; | |||
There was a problem hiding this comment.
Note: this change is because the IntoFuture trait no longer exists.
tomaka
commented
Jul 19, 2019
| if let Some(pool) = &mut pool { | ||
| // TODO: this expect() can be removed once | ||
| // https://github.com/rust-lang-nursery/futures-rs/pull/1750 is merged and deployed | ||
| pool.spawn(future).expect("ThreadPool can never fail to spawn tasks; QED"); |
There was a problem hiding this comment.
Note that it is unfortunately no longer possible to properly recover from task spawning errors.
I went for creating a local ThreadPool here so that we are guaranteed to never have any error (as in: if creating the pool succeeds, then spawning the task will also succeed).
In the future, we should probably pass some task spawning utility by parameter, but the idiomatic way to do that remains to be discovered.
This was referenced Jul 22, 2019
Merged
|
I'd prefer to merge the BABE PRs first, if that's OK. |
gavofyork
approved these changes
Jul 25, 2019
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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 had this PR almost ready for a long time, was just waiting for #3117 to be merged.
Switches
consensus/*to new futures.Most notably, however,
start_auraandstart_babestill return an old future (by building a new future and putting the compatibility layer around), so that we avoid breaking everything.cc #3099