Add CLI args and env var support#98
Conversation
|
👋 Thanks for assigning @benthecarman as a reviewer! |
Anyitechs
force-pushed
the
config-env
branch
2 times, most recently
from
cc9d26c to
4d0569f
Compare
|
🔔 1st Reminder Hey @benthecarman! This PR has been waiting for your review. |
Adds support for configuring the node via CLI arguments and environment variables, allowing runtime overrides of the configuration file. - Added `clap` dependency for argument parsing. - Implemented layered config loading: config file (full set of options) + environment variables + CLI arguments. Env vars and CLI args override values from the config file when present. - Implemented `ConfigBuilder` to handle partial state and merging. - Added comprehensive unit tests for precedence and validation logic. - Updated README with usage instructions and explanation of config precedence. Co-authored-by: moisesPomilio <93723302+moisesPompilio@users.noreply.github.com>
README.md
Outdated
| cargo run --bin ldk-server | ||
| ``` | ||
|
|
||
| - Using CLI arguments (all optional): |
There was a problem hiding this comment.
this is calling the cli, not running the server with command line args
ldk-server/src/util/config.rs
Outdated
| } | ||
|
|
||
| #[derive(Debug)] | ||
| #[derive(Debug, PartialEq)] |
ldk-server/src/util/config.rs
Outdated
| #[cfg(any(feature = "events-rabbitmq", feature = "experimental-lsps2-support"))] | ||
| return Err(io::Error::new( | ||
| io::ErrorKind::InvalidInput, | ||
| format!( | ||
| "To use the `{}` feature, you must provide a configuration file.", | ||
| if cfg!(feature = "events-rabbitmq") { | ||
| "events-rabbitmq" | ||
| } else { | ||
| "experimental-lsps2-support" | ||
| } | ||
| ), | ||
| )); | ||
| } |
There was a problem hiding this comment.
Why is this the else case? If no config file is specified in the args, we should instead look for one in the default data dir
There was a problem hiding this comment.
Done. Now checking the default data dir, if no config file is passed in the args.
There was a problem hiding this comment.
this else case is still unchanged
ldk-server/src/main.rs
Outdated
| Ok(config) => config, | ||
| Err(e) => { | ||
| eprintln!("Invalid configuration file: {}", e); | ||
| eprintln!("Invalid configuration: {}", e); |
There was a problem hiding this comment.
| eprintln!("Invalid configuration: {}", e); | |
| eprintln!("Invalid configuration: {e}"); |
|
|
||
| let mut ldk_node_config = Config::default(); | ||
| let config_file = match load_config(&config_path) { | ||
| let config_file = match load_config(&args_config) { |
There was a problem hiding this comment.
Config path and the args above is never used now. We should be able to remove that logic as we use clap now, but we still should check for the default config file
|
Addressed all comments here f57f489 |
|
🔔 1st Reminder Hey @benthecarman! This PR has been waiting for your review. |
3 participants
Continues the work started in #69 by @moisesPompilio adding support for configuring the node via CLI arguments and environment variables, allowing runtime overrides of the configuration file.
I introduced
ConfigBuilderto handle partial state and merging of the configuration fields, which significantly cleaned up theload_configlogic.