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

Cleaner binary shutdown system #8284

Merged
merged 4 commits into from
Apr 4, 2018
Merged

Conversation

tomaka
Copy link
Contributor

@tomaka tomaka commented Apr 1, 2018

Part of #4842

Refactors a bit of the code of parity/run.rs in order to have a better separation between the code that runs the client and the code that handles the ctrl+c and the can_restart option.

@tomaka tomaka added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Apr 1, 2018
@5chdn 5chdn added this to the 1.11 milestone Apr 3, 2018
/// Set a closure to call when the client wants to be restarted.
///
/// The parameter passed to the callback is the name of the new chain spec to use after
/// the restart.
Copy link
Collaborator

Choose a reason for hiding this comment

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

finally a proper description of a function! 👍

fn execute_impl<Cr, Rr>(cmd: RunCmd, logger: Arc<RotatingLogger>, on_client_rq: Cr,
on_updater_rq: Rr) -> Result<RunningClient, String>
where Cr: Fn(String) + 'static + Send,
Rr: Fn() + 'static + Send
Copy link
Collaborator

@debris debris Apr 4, 2018

Choose a reason for hiding this comment

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

do you know why doesn't updater care about the spec? isn't it a bug?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know how the updater works unfortunately

Copy link
Collaborator

Choose a reason for hiding this comment

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

you're right, it's not related to this pr. I'll ask andre

Copy link
Collaborator

Choose a reason for hiding this comment

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

@andresilva told me that everything's good :)

I think it's fine because the updater gets all its info from the operations contract, so it doesn't need the chainspec
so only the client needs to be updated with the new spec, and the updater will use the updated client to interact with the operations contract

@debris debris added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Apr 4, 2018
@5chdn 5chdn merged commit e12a515 into openethereum:master Apr 4, 2018
@tomaka tomaka deleted the cleaner-shutdown branch April 4, 2018 09:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants