Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Make Proposer instantiation potentially async. #4630

Merged
merged 5 commits into from
Jan 15, 2020
Merged

Conversation

rphmeier
Copy link
Contributor

Needed for an upcoming Polkadot refactor and is strictly more general. This API was always intended to be used in an async context, so it doesn't break any underlying assumptions.

@rphmeier rphmeier added the A0-please_review Pull request needs code review. label Jan 15, 2020
@rphmeier
Copy link
Contributor Author

rphmeier commented Jan 15, 2020

(tiny Polkadot PR incoming - will be followed up by the actual refactor)

@rphmeier rphmeier force-pushed the rh-environment-async branch from 241162b to 4ae8e92 Compare January 15, 2020 14:21
RecordProof::Yes,
));

let new_block = futures::executor::block_on(proposing)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May I suggest something like this instead?

Suggested change
let new_block = futures::executor::block_on(proposing)
let new_block = futures::executor::block_on(async move {
let proposer = proposer_factory.init(&parent_header).await;
proposer.propose(
inherent_data,
digest,
std::time::Duration::from_secs(1),
RecordProof::Yes,
).await
});

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still need to get the async/await into my bloodstream :)

@rphmeier rphmeier merged commit 1dafa60 into master Jan 15, 2020
@rphmeier rphmeier deleted the rh-environment-async branch January 15, 2020 20:09
rphmeier added a commit that referenced this pull request Jan 15, 2020
* Make Proposer instantiation potentially async.

* fix node-service test

* fix basic-authority doc-test

* only block once on futures in test

* use async/await
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants