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

Implement Clap derive #5900

Open
2 of 8 tasks
eserilev opened this issue Jun 6, 2024 · 0 comments
Open
2 of 8 tasks

Implement Clap derive #5900

eserilev opened this issue Jun 6, 2024 · 0 comments
Assignees

Comments

@eserilev
Copy link
Collaborator

eserilev commented Jun 6, 2024

This issue will track the tasks for migrating Lighthouse from clap builder to derive.

Clap derive offers some nice features including

  • type casted flag definitions, no need to do any post processing to the flag values i.e. converting strings to other types
  • easier to read flag definitions

Clap derive is a bit less flexible than the builder, but its possible to combine the builder with derive if needed. All of our flag definitions today are fairly "standard", so derive shouldn't cause any issues.

Another nice bonus with clap derive is it should make ingesting a config at startup easy. Its fairly trivial to map a json,yaml,whatever file to a struct. And the clap derive flag definitions are structs. An example might make things a bit clearer.

Take this function that parses cli arguments and returns some config that Lighthouse then uses to do stuff

fn parse_client_config<E: EthSpec>(
    cli_args: &ArgMatches,
    _env: &Environment<E>,
) -> Result<ClientConfig, String>

With clap derive, instead of passing in the cli args, we pass in the struct that defines those cli args

fn parse_client_config<E: EthSpec>(
    database_manager_config: &DatabaseManager,
    _env: &Environment<E>,
) -> Result<ClientConfig, String>

It then becomes straight foward to write some code that maps a provided yaml,json,etc file to an instance of DatabaseManager and feed that into the same parse_client_config function. Were now re-using the same code paths that ingest cli params from the command line to ingest cli params from a file.

Tasks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant