-
Notifications
You must be signed in to change notification settings - Fork 247
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
Custom spawn_tasks
#2973
Custom spawn_tasks
#2973
Conversation
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.
Do you mind adding context?
Context is basically in the second commit, PR description and partly in paritytech/polkadot-sdk#5364 mentioned in TODO We don't need Substrate's prometheus server, we don't want its RPC server in some cases and we also don't want |
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.
A small refactoring request - otherwise looks OK.
crates/subspace-service/src/lib.rs
Outdated
@@ -31,6 +31,7 @@ pub mod dsn; | |||
mod metrics; | |||
pub(crate) mod mmr; | |||
pub mod rpc; | |||
mod spawn_tasks; |
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.
Consider changing to task_spawner
: spawn_tasks::spawn_tasks
looks awkward.
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.
Renamed, though it is really a bad name in Substrate, I wouldn't call that function spawn_tasks
in the first place.
One more thing: this finally allows us to customize informer, which we'll extend to show Subspace networking stats in addition to Substrate networking as well as various DSN sync stages, etc. |
Finally we can do this.
Still can't remove
sp-session
dependency due to RPC layer requiring it (alongside with keystore which consensus chain doesn't have), I'm still thinking what to do about it without copy-pasting a bunch of non-trivial boilerplate.Code contributor checklist: