feat: add arguments from run cmd to init cmd#129
Conversation
695928a to
2eff9d3
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds the ability to override configuration values via command-line arguments during the init command. Previously, these arguments were only available in the run command. This change enables the Quickstart/local setup to configure gateway addresses and exclude CIDR ranges during initialization.
Key Changes
- Added network configuration arguments (gateway, listen, external addresses, relay circuit, exclude CIDR) to the
initcommand across all components - Modified the initialization logic to apply command-line arguments to the generated config via
Serialized::defaults(&args) - Added component-specific arguments (socket/work-dir for worker, certificates for gateway, dataset-path already existed for data)
Reviewed Changes
Copilot reviewed 5 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/worker/src/cli.rs | Added network configuration, socket, work-dir, and exclude-cidr arguments to Init command |
| crates/worker/src/bin/hypha-worker.rs | Modified init handler to apply CLI arguments to config generation |
| crates/scheduler/src/cli.rs | Added network configuration and exclude-cidr arguments to Init command |
| crates/scheduler/src/bin/hypha-scheduler.rs | Modified init handler to apply CLI arguments to config generation |
| crates/gateway/src/cli.rs | Added certificate paths, listen/external addresses, and exclude-cidr arguments to Init command |
| crates/gateway/src/bin/hypha-gateway.rs | Modified init handler to apply CLI arguments to config generation |
| crates/data/src/cli.rs | Added gateway, listen addresses, and exclude-cidr arguments to Init command |
| crates/data/src/bin/hypha-data.rs | Modified init handler to apply CLI arguments to config generation |
| crates/data/Cargo.toml | Added hypha-network and libp2p dependencies for new CLI types |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// --gateway /ip4/203.0.113.10/tcp/8080/ | ||
| /// --gateway /dns4/gateway.hypha.example/tcp/443/ |
There was a problem hiding this comment.
The gateway example comments are incomplete and inconsistent with worker/data examples. In worker and data, the gateway examples include the peer ID component (/p2p/12D3KooWAbc...), which is mentioned as required in the documentation. However, the scheduler examples are missing this component. For consistency and completeness, these should include the peer ID as well.
| /// --gateway /ip4/203.0.113.10/tcp/8080/ | |
| /// --gateway /dns4/gateway.hypha.example/tcp/443/ | |
| /// --gateway /ip4/203.0.113.10/tcp/8080/p2p/12D3KooWAbc... | |
| /// --gateway /dns4/gateway.hypha.example/tcp/443/p2p/12D3KooWAbc... |
| /// Filters out peer addresses matching these ranges before adding to the DHT. | ||
| /// Examples: 10.0.0.0/8, fc00::/7 |
There was a problem hiding this comment.
Missing space after "DHT." in the comment. Should be "DHT. Examples:" with proper spacing between sentences for consistency with other similar comments in the codebase.
| /// Filters out peer addresses matching these ranges before adding to the DHT. | |
| /// Examples: 10.0.0.0/8, fc00::/7 | |
| /// Filters out peer addresses matching these ranges before adding to the DHT. Examples: 10.0.0.0/8, fc00::/7 |
| hypha-network.workspace = true | ||
| libp2p.workspace = true |
There was a problem hiding this comment.
These dependencies (hypha-network and libp2p) are added to [build-dependencies] but they're used in the main source code (src/cli.rs), not in build scripts. They should be added to the regular [dependencies] section instead of [build-dependencies]. Build dependencies are only available to build scripts (build.rs), not to the main application code.
| hypha-network.workspace = true | |
| libp2p.workspace = true |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
adding ability to override more values for the configs.. we need this for the Quickstart/local setup to be able to set gateway address and exclude cidr.