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

Allow using any polkadot client instead of enum Client#1575

Merged
15 commits merged intomasterfrom
cecton-allow-using-any-polkadot-client
Aug 13, 2020
Merged

Allow using any polkadot client instead of enum Client#1575
15 commits merged intomasterfrom
cecton-allow-using-any-polkadot-client

Conversation

@cecton
Copy link
Contributor

@cecton cecton commented Aug 12, 2020

cecton added 8 commits August 12, 2020 18:29
@cecton
WIP
ac4e6c4 
Forked at: e916423
Parent branch: origin/rococo-branch
@cecton
WIP
5527eaa 
Forked at: e916423
Parent branch: origin/rococo-branch
@cecton
WIP
47f184e 
Forked at: e916423
Parent branch: origin/rococo-branch
@cecton
WIP
f60375c 
Forked at: e916423
Parent branch: origin/rococo-branch
@cecton
WIP
99bbf74 
Forked at: e916423
Parent branch: origin/rococo-branch
@cecton
WIP
43512e8 
Forked at: e916423
Parent branch: origin/rococo-branch
@cecton
WIP
ae7082c 
Forked at: e916423
Parent branch: origin/rococo-branch
@cecton
WIP
fb78f0b 
Forked at: e916423
Parent branch: origin/rococo-branch
@cecton cecton force-pushed the cecton-allow-using-any-polkadot-client branch from 14590ff to fb78f0b Compare August 12, 2020 16:29
cecton
cecton commented Aug 12, 2020
View reviewed changes
cecton
cecton commented Aug 12, 2020
View reviewed changes
cecton and others added 3 commits August 12, 2020 18:36
@cecton
Apply suggestions from code review
846e23f
@cecton
WIP
5caaaeb 
Forked at: e916423
Parent branch: origin/rococo-branch
@cecton
WIP
0070028 
Forked at: e916423
Parent branch: origin/rococo-branch
cecton
cecton commented Aug 13, 2020
View reviewed changes
service/src/client.rs Outdated
Comment on lines 121 to 125
/// Yet Another ExecuteWithClient
pub trait YaExecuteWithClient {
/// Execute the given something with the client.
fn execute_with<T: ExecuteWithClient>(&self, t: T) -> T::Output;
}
Copy link
Contributor Author

@cecton cecton Aug 13, 2020

Choose a reason for hiding this comment

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

I'm not sure how to call it... ClientExecutor? ClientExecutionContext? @bkchr

@cecton cecton marked this pull request as ready for review August 13, 2020 07:49
@github-actions github-actions bot added the A0-please_review Pull request needs code review. label Aug 13, 2020
@cecton
WIP
b66861c 
Forked at: e916423
Parent branch: origin/rococo-branch
bkchr
bkchr approved these changes Aug 13, 2020
View reviewed changes
collator/src/lib.rs
where
SP: SpawnNamed + Clone + Send + Sync + 'static;
SP: SpawnNamed + Clone + Send + Sync + 'static,
Backend: BackendT<Block>,
Copy link
Member

@bkchr bkchr Aug 13, 2020

Choose a reason for hiding this comment

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

I was sooo happy that we got rid off it :P But yeah, this will also make some code easier in Cumulus 👍

Copy link
Contributor Author

@cecton cecton Aug 13, 2020

Choose a reason for hiding this comment

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

I AM SO SORRY

service/src/client.rs Outdated
}

/// Yet Another ExecuteWithClient
pub trait YaExecuteWithClient {
Copy link
Member

@bkchr bkchr Aug 13, 2020

Choose a reason for hiding this comment

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

Maybe ClientHandle?

With a much better documentation on what it does and why we need it? Something like:

/// A handle to a Polkadot client instance
///
/// The Polkadot service supports multiple different runtimes, like
/// Westend, Polkadot itself, etc. As each runtime has a specialized
/// client, we need to hide them behind a trait. This is this trait. 
///
/// When wanting to work with the inner client, you need to use [`execute_with`](ClientHandle::execute_with).
/// See [`ExecuteWithClient`] for more information.

Copy link
Contributor Author

@cecton cecton Aug 13, 2020

Choose a reason for hiding this comment

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

omg I was not expecting that much 😆 🙏

cecton added 3 commits August 13, 2020 10:12
@cecton
CLEANUP
96cdf51 
Forked at: e916423
Parent branch: origin/rococo-branch
@cecton
link in doc
5c4f5ca
@cecton
doc
12cbf02
@cecton
Copy link
Contributor Author

cecton commented Aug 13, 2020

bot merge

@ghost
Copy link

ghost commented Aug 13, 2020

Missing process info; check that the PR belongs to a project column.

@cecton cecton added 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 Aug 13, 2020
@cecton
Copy link
Contributor Author

cecton commented Aug 13, 2020

bot merge

@ghost
Copy link

ghost commented Aug 13, 2020

Waiting for commit status.

@ghost ghost merged commit 3b62357 into master Aug 13, 2020
@ghost ghost deleted the cecton-allow-using-any-polkadot-client branch August 13, 2020 08:40
cecton added a commit that referenced this pull request Aug 13, 2020
@cecton
Allow using any polkadot client instead of enum Client (#1575)
a02ebf7 
* WIP

Forked at: e916423
Parent branch: origin/rococo-branch

* WIP

Forked at: e916423
Parent branch: origin/rococo-branch

* WIP

Forked at: e916423
Parent branch: origin/rococo-branch

* WIP

Forked at: e916423
Parent branch: origin/rococo-branch

* WIP

Forked at: e916423
Parent branch: origin/rococo-branch

* WIP

Forked at: e916423
Parent branch: origin/rococo-branch

* WIP

Forked at: e916423
Parent branch: origin/rococo-branch

* WIP

Forked at: e916423
Parent branch: origin/rococo-branch

* Apply suggestions from code review

* WIP

Forked at: e916423
Parent branch: origin/rococo-branch

* WIP

Forked at: e916423
Parent branch: origin/rococo-branch

* WIP

Forked at: e916423
Parent branch: origin/rococo-branch

* CLEANUP

Forked at: e916423
Parent branch: origin/rococo-branch

* link in doc

* doc
ordian added a commit that referenced this pull request Aug 14, 2020
@ordian
Merge branch 'master' into ao-prometheus-metrics
85b1ba5 
* master:
  Make parachain validation wasm executor functional (#1574)
  Use async test helper to simplify node testing (#1578)
  guide: validation data refactoring (#1576)
  Remove v0 parachains runtime (#1501)
  [CI] Add github token to generate-release-text (#1581)
  Allow using any polkadot client instead of enum Client (#1575)
  service/src/lib: Update authority discovery construction (#1563)
  Update .editorconfig to what we have in practice (#1545)
  Companion PR for substrate #6672 (#1560)
  pre-redenomination tockenSymbol change (#1561)
ordian added a commit that referenced this pull request Aug 17, 2020
@ordian
Merge branch 'master' into ao-job-manager-dont-bring-down-subsystem-o…
91f1024 
…n-race-condition

* master:
  Companion PR for #6862 (#1564)
  implement collation generation subsystem (#1557)
  Add spawn_blocking to SubsystemContext (#1570)
  Companion PR for #6846 (#1568)
  overseer: add a test to ensure all subsystem receive msgs (#1590)
  Implementer's Guide: Flesh out more details for upward messages (#1556)
  Make parachain validation wasm executor functional (#1574)
  Use async test helper to simplify node testing (#1578)
  guide: validation data refactoring (#1576)
  Remove v0 parachains runtime (#1501)
  [CI] Add github token to generate-release-text (#1581)
  Allow using any polkadot client instead of enum Client (#1575)
  service/src/lib: Update authority discovery construction (#1563)
ordian added a commit that referenced this pull request Aug 17, 2020
@ordian
Merge branch 'master' into ao-refactor-primitives
32fefa6 
* master:
  overseer: fix build (#1596)
  Companion PR for #6862 (#1564)
  implement collation generation subsystem (#1557)
  Add spawn_blocking to SubsystemContext (#1570)
  Companion PR for #6846 (#1568)
  overseer: add a test to ensure all subsystem receive msgs (#1590)
  Implementer's Guide: Flesh out more details for upward messages (#1556)
  Make parachain validation wasm executor functional (#1574)
  Use async test helper to simplify node testing (#1578)
  guide: validation data refactoring (#1576)
  Remove v0 parachains runtime (#1501)
  [CI] Add github token to generate-release-text (#1581)
  Allow using any polkadot client instead of enum Client (#1575)
  service/src/lib: Update authority discovery construction (#1563)
  Update .editorconfig to what we have in practice (#1545)
  Companion PR for substrate #6672 (#1560)
  pre-redenomination tockenSymbol change (#1561)
cecton added a commit that referenced this pull request Sep 4, 2020
@cecton
Allow using any polkadot client instead of enum Client (#1575)
86850fa 
* WIP

Forked at: e916423
Parent branch: origin/rococo-branch

* WIP

Forked at: e916423
Parent branch: origin/rococo-branch

* WIP

Forked at: e916423
Parent branch: origin/rococo-branch

* WIP

Forked at: e916423
Parent branch: origin/rococo-branch

* WIP

Forked at: e916423
Parent branch: origin/rococo-branch

* WIP

Forked at: e916423
Parent branch: origin/rococo-branch

* WIP

Forked at: e916423
Parent branch: origin/rococo-branch

* WIP

Forked at: e916423
Parent branch: origin/rococo-branch

* Apply suggestions from code review

* WIP

Forked at: e916423
Parent branch: origin/rococo-branch

* WIP

Forked at: e916423
Parent branch: origin/rococo-branch

* WIP

Forked at: e916423
Parent branch: origin/rococo-branch

* CLEANUP

Forked at: e916423
Parent branch: origin/rococo-branch

* link in doc

* doc
@cecton cecton mentioned this pull request Sep 4, 2020
Merged
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@bkchr bkchr bkchr approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

A0-please_review Pull request needs code review. 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.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@cecton @bkchr @paritytech-ci