Skip to content
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

Refactor sc_service::config::Configuration struct #2897

Closed
2 tasks done
nazar-pc opened this issue Jan 10, 2024 · 0 comments · Fixed by #5364
Closed
2 tasks done

Refactor sc_service::config::Configuration struct #2897

nazar-pc opened this issue Jan 10, 2024 · 0 comments · Fixed by #5364
Labels
I5-enhancement An additional feature request.

Comments

@nazar-pc
Copy link
Contributor

Is there an existing issue?

  • I have searched the existing issues

Experiencing problems? Have you tried our Stack Exchange first?

  • This is not a support question.

Motivation

sc_service::config::Configuration is an extremely annoying right now.

It contains a huge number of stuff in general, many of its related fields are in the flat list (for example there are 7 RPC-related fields, also rpc_addr and rpc_port are separate, while only the first one should be needed).

Moreover, it is abused by many components that instead of having arguments that they actually need, require the whole Configuration to be constructed. One example of this is new_wasm_executor that takes &Configuration as an argument, but only needs 4 fields that could have been a part of a small data structure:

  • default_heap_pages
  • wasm_method
  • max_runtime_instances
  • runtime_cache_size

Request

Group many of the fields of sc_service::config::Configuration in a logical way (kind of like network is grouped right now) and change most of the usage to only require specific fields instead of the whole Configuration data structure.

Solution

No response

Are you willing to help with this request?

Yes!

@nazar-pc nazar-pc added the I5-enhancement An additional feature request. label Jan 10, 2024
@nazar-pc nazar-pc changed the title Refactor Configuration struct Refactor sc_service::config::Configuration struct Jan 10, 2024
github-merge-queue bot pushed a commit that referenced this issue Sep 2, 2024
This improves `sc-service` API by not requiring the whole
`&Configuration`, using specific configuration options instead.
`RpcConfiguration` was also extracted from `Configuration` to group all
RPC options together.

We don't use Substrate's CLI and would rather not use `Configuration`
either, but some key public functions require it even though they
ignored most of the fields anyway.

`RpcConfiguration` is very helpful not just for consolidation of the
fields, but also to finally make RPC optional for our use case, while
Substrate still runs RPC server on localhost even if listening address
is explicitly set to `None`, which is annoying (and I suspect there is a
reason for it, so didn't want to change the default just yet).

While this is a breaking change, most developers will not notice it if
they use higher-level APIs.

Fixes #2897

---------

Co-authored-by: Niklas Adolfsson <[email protected]>
x3c41a pushed a commit that referenced this issue Sep 4, 2024
This improves `sc-service` API by not requiring the whole
`&Configuration`, using specific configuration options instead.
`RpcConfiguration` was also extracted from `Configuration` to group all
RPC options together.

We don't use Substrate's CLI and would rather not use `Configuration`
either, but some key public functions require it even though they
ignored most of the fields anyway.

`RpcConfiguration` is very helpful not just for consolidation of the
fields, but also to finally make RPC optional for our use case, while
Substrate still runs RPC server on localhost even if listening address
is explicitly set to `None`, which is annoying (and I suspect there is a
reason for it, so didn't want to change the default just yet).

While this is a breaking change, most developers will not notice it if
they use higher-level APIs.

Fixes #2897

---------

Co-authored-by: Niklas Adolfsson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I5-enhancement An additional feature request.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant