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

NetworkService::new starts the network #462

Merged
merged 3 commits into from
Aug 6, 2018

Conversation

tomaka
Copy link
Contributor

@tomaka tomaka commented Jul 30, 2018

Instead of creating the service with new(), then calling start(), we do both at once.
This makes the code of NetworkService much more straight-forward, and is IMO better programming practices.

If we ever want to be able to stop the network, I suggest using a Option<NetworkService> instead.

@tomaka tomaka added A0-please_review Pull request needs code review. M4-core labels Jul 30, 2018
@@ -276,8 +273,6 @@ impl<Components> Drop for Service<Components> where Components: components::Comp
fn drop(&mut self) {
debug!(target: "service", "Substrate service shutdown");

self.network.stop_network();
Copy link
Member

@arkpar arkpar Jul 31, 2018

Choose a reason for hiding this comment

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

Doest the shutdown of an authority node on Ctrl-C work OK after this? IIRC this was required so that network would terminate Future streams it exposes and that would release the BFT process and allow it to terminate.

Copy link
Contributor

Choose a reason for hiding this comment

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

drop implements that same behaviour, so drop(self.network.take()); in the following line should have the same effect.

@gnunicorn gnunicorn added A8-looksgood and removed A0-please_review Pull request needs code review. labels Aug 6, 2018
@gnunicorn gnunicorn merged commit 35f1af4 into paritytech:master Aug 6, 2018
@tomaka tomaka deleted the network-always-started branch August 6, 2018 09:59
dvdplm added a commit that referenced this pull request Aug 7, 2018
* master:
  Fix some formatting grumbles (#501)
  remove the extra ? in the license header (#500)
  NetworkService::new starts the network (#462)
  Availability/Extrinsic store (#465)
  Add doc to install a specific tagged version (#497)
lamafab pushed a commit to lamafab/substrate that referenced this pull request Jun 16, 2020
* update substrate to latest polkadot-master

* fix test runtime
helin6 pushed a commit to boolnetwork/substrate that referenced this pull request Jul 25, 2023
* Update scale-info and parity-scale-codec requirements

* Update CHANGELOG

* Update frame-metadata

* Update bitvec

* Fix test

* Update bitvec event decoding

* More bitvec updates

* Update substrate primitives dependencies

* Fix up bitvec errors and decode error

* Fix up bitvec errors

* Update polkadot codegen
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants