connected disconnected state removal #3803

drahnr · 2021-09-07T08:38:35Z

Removes the concept of a disconnected or connected overseer, which doesn't really have any practical meaning.

splits the fn new_partial into multiple
introduce is_relay_chain and use that to selection which chain selection to use, closes overseer: remove connect/disconnect state of the Handle #3777
unify the approach of setting up a struct Overseer, since there were two code paths achieving the same thing, now it's unified to the builder pattern, AllSubsystemsGen and AllSubsystem<_,...> are gone for good, closes [overseer] remove AllSubsystems and proc-macro AllSubsystemGen #3773

node/service/src/lib.rs

Obsoletes `Overseer::new`, `AllSubsystemsGen` derive macro, `AllSubsystems`.

drahnr · 2021-09-08T15:53:23Z

Since we want to avoid all ways of possible pitfallls around the setup of an overseer, this will also unify the setup path to the OverseerBuilder with a few convenience fn helpers, and as a consequence will also cover #3773

…onnected-disconnected-removal

ordian

Looks like a good simplification overall!

node/overseer/overseer-gen/proc-macro/src/impl_overseer.rs

node/service/src/lib.rs

node/overseer/src/dummy.rs

ordian

Looks good, just a few questions.

ordian · 2021-09-10T10:09:06Z

node/overseer/overseer-gen/proc-macro/src/impl_builder.rs

@@ -35,6 +68,12 @@ pub(crate) fn impl_builder(info: &OverseerInfo) -> proc_macro2::TokenStream {
 		.iter()
 		.map(|subsystem_name| format_ident!("{}_with", subsystem_name))


where is it used?

I think this can be removed again in a future PR, this was added when I was under the impression that we could get away without OverseerConnector which was wrong ultimately.

I'll remove this in a separate PR.

node/service/src/relay_chain_selection.rs

cli/src/lib.rs

…d-disconnected-removal

…onnected-disconnected-removal

node/service/src/overseer.rs

…onnected-disconnected-removal

node/service/src/lib.rs

node/service/src/overseer.rs

node/service/src/relay_chain_selection.rs

eskimor · 2021-09-14T14:50:04Z

The diff is quite a bit to digest for me at least - trying on Rococo before merge, might be a good idea.

drahnr · 2021-09-14T15:15:25Z

The diff is quite a bit to digest for me at least - trying on Rococo before merge, might be a good idea.

Fair point, it got larger than anticipated and touches a lot of essentials.

…d-disconnected-removal

node/overseer/examples/minimal-example.rs

node/service/src/lib.rs

drahnr added 3 commits September 6, 2021 18:38

X

0c576f3

fixup proc macro

9b10894

remove connect disconnect state

1a90b59

drahnr self-assigned this Sep 7, 2021

drahnr added A3-in_progress Pull request is in progress. No review needed at this stage. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Sep 7, 2021

drahnr added 2 commits September 7, 2021 10:50

cleanup warnings

04b48ea

split new partial

8f738dc

rphmeier reviewed Sep 7, 2021

View reviewed changes

node/service/src/lib.rs Show resolved Hide resolved

drahnr added 5 commits September 8, 2021 10:45

cleanup: overseer residue

9457019

spellcheck

b5a3f5f

fixin

b9d1cf1

groundwork to obsolete Overseer::new and AllSubsystemsGen proc-macro

2ded22d

Now all malus & tests can be ported to the builder pattern.

ed10d78

Obsoletes `Overseer::new`, `AllSubsystemsGen` derive macro, `AllSubsystems`.

drahnr added 6 commits September 8, 2021 17:54

spellcheck

f895214

adjust tests, minor fixes

4865bdf

remove derive macro AllSubsystemsGen

6cac23e

add forgotten file dummy.rs

f95761d

remove residue

d212767

good news everyone!

00015d4

drahnr added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Sep 9, 2021

drahnr added 3 commits September 9, 2021 10:20

spellcheck

f7990be

spellcheck

9ef9b2e

Merge remote-tracking branch 'origin/master' into bernhard-overseer-c…

363231f

…onnected-disconnected-removal

ordian linked an issue Sep 9, 2021 that may be closed by this pull request

[overseer] remove AllSubsystems and proc-macro AllSubsystemGen #3773

Closed

ordian reviewed Sep 9, 2021

View reviewed changes

node/overseer/overseer-gen/proc-macro/src/impl_overseer.rs Show resolved Hide resolved

node/service/src/lib.rs Outdated Show resolved Hide resolved

node/overseer/src/dummy.rs Outdated Show resolved Hide resolved

Lldenaurois reviewed Sep 9, 2021

View reviewed changes

node/overseer/src/dummy.rs Show resolved Hide resolved

ordian approved these changes Sep 10, 2021

View reviewed changes

drahnr added 2 commits September 10, 2021 15:57

fixup docs

de6f89a

reduce import

1eda912

drahnr requested review from Lldenaurois and eskimor September 10, 2021 14:01

drahnr added 5 commits September 10, 2021 16:23

Merge remote-tracking branch 'origin' into bernhard-overseer-connecte…

05f3155

…d-disconnected-removal

spellcheck

1ef6d3b

Merge remote-tracking branch 'origin/master' into bernhard-overseer-c…

8334908

…onnected-disconnected-removal

chore: fmt

6acee9f

hide basics

2f60f54

drahnr requested a review from rphmeier September 13, 2021 15:38

eskimor reviewed Sep 14, 2021

View reviewed changes

node/service/src/overseer.rs Show resolved Hide resolved

Merge remote-tracking branch 'origin/master' into bernhard-overseer-c…

9e58be3

…onnected-disconnected-removal

eskimor reviewed Sep 14, 2021

View reviewed changes

node/service/src/lib.rs Show resolved Hide resolved

node/service/src/overseer.rs Show resolved Hide resolved

node/service/src/relay_chain_selection.rs Outdated Show resolved Hide resolved

drahnr commented Sep 14, 2021

View reviewed changes

node/service/src/relay_chain_selection.rs Outdated Show resolved Hide resolved

Update node/service/src/relay_chain_selection.rs

be948fd

drahnr added the A1-needsburnin label Sep 14, 2021

drahnr removed the A1-needsburnin label Sep 16, 2021

drahnr added 2 commits September 16, 2021 11:06

Merge remote-tracking branch 'origin' into bernhard-overseer-connecte…

a10f0f1

…d-disconnected-removal

chore: fmt

e84e6f4

drahnr added the A1-needsburnin label Sep 16, 2021

drahnr added 2 commits September 16, 2021 11:20

chore: spellcheck / nlprules

eee560a

fixup malus variant-a

2e1257f

This was referenced Sep 16, 2021

remove connected disconnected state only #3868

Merged

remove AllSubsystems and AllSubsystemsGen types #3874

Merged

Lldenaurois reviewed Sep 17, 2021

View reviewed changes

node/overseer/examples/minimal-example.rs Show resolved Hide resolved

Lldenaurois approved these changes Sep 17, 2021

View reviewed changes

node/service/src/lib.rs Show resolved Hide resolved

node/service/src/lib.rs Show resolved Hide resolved

drahnr closed this Sep 20, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

connected disconnected state removal #3803

connected disconnected state removal #3803

drahnr commented Sep 7, 2021 •

edited

Loading

drahnr commented Sep 8, 2021 •

edited

Loading

ordian left a comment

ordian left a comment

ordian Sep 10, 2021

drahnr Sep 10, 2021

eskimor commented Sep 14, 2021

drahnr commented Sep 14, 2021

		@@ -35,6 +68,12 @@ pub(crate) fn impl_builder(info: &OverseerInfo) -> proc_macro2::TokenStream {
		.iter()
		.map(\|subsystem_name\| format_ident!("{}_with", subsystem_name))

connected disconnected state removal #3803

connected disconnected state removal #3803

Conversation

drahnr commented Sep 7, 2021 • edited Loading

drahnr commented Sep 8, 2021 • edited Loading

ordian left a comment

Choose a reason for hiding this comment

ordian left a comment

Choose a reason for hiding this comment

ordian Sep 10, 2021

Choose a reason for hiding this comment

drahnr Sep 10, 2021

Choose a reason for hiding this comment

eskimor commented Sep 14, 2021

drahnr commented Sep 14, 2021

drahnr commented Sep 7, 2021 •

edited

Loading

drahnr commented Sep 8, 2021 •

edited

Loading