-
Notifications
You must be signed in to change notification settings - Fork 72
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(boost): add ServiceHandle future #57
Conversation
3a9f2c8
to
3b19a3a
Compare
split
this has the benefit that after calling the ServiceHandle is currently duplicated. imo this fine |
d3656e1
to
e81dcb3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry for the delay on this! I haven't used pin_project
before so wanted to be able to give it some attention before merging it in
this is excellent and thanks for the contribution :)
this change is strictly better imo as the API now gives better control over when and how the various tasks are constructed/orchestrated -- we can see this where you drop the arbitrary sleep
from the integration test code
d9817c3
to
e81dcb3
Compare
e81dcb3
to
63c8d44
Compare
thanks for this! I like adding the |
rename
async fn Service::run
tofn Service::spawn() -> ServiceHandle
add
ServiceHandle
Future that waits for both task to finishsince this launches the server this could be improved to wait until the server is up, but can tackle that after looking at
BlindedBlockProviderServer
, so perhaps this PR is premature