Add support for generic clients #49
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gregcusack
force-pushed
the
generic-client-v3
branch
from
6ba9f16 to
d0a089c
Compare
joncinque
reviewed
Aug 2, 2024
src/lib.rs
Outdated
| @@ -314,3 +322,13 @@ pub fn check_directory(path: &Path, description: &str) -> Result<(), Box<dyn std | |||
| } | |||
| Ok(()) | |||
| } | |||
|
|
|||
| pub fn validate_docker_image(image: String) -> Result<(), String> { | |||
There was a problem hiding this comment.
nit: can this take a &str instead since it's validating from the command line?
src/main.rs
Outdated
| .default_value("0") | ||
| .help("Wait for `delay-start` seconds after all validators are deployed to deploy client. | ||
| Use case: If client needs to connect to a specific node, but that node hasn't fully deployed yet | ||
| the client may no be able to resolve the node's endpoint. `--delay-start` is used to wait so |
There was a problem hiding this comment.
micro-nit
Suggested change
| the client may no be able to resolve the node's endpoint. `--delay-start` is used to wait so | |
| the client may not be able to resolve the node's endpoint. `--delay-start` is used to wait so |
src/main.rs
Outdated
Comment on lines
267
to
274
| .arg( | ||
| Arg::with_name("client_duration_seconds") | ||
| .long("client-duration-seconds") | ||
| .takes_value(true) | ||
| .default_value("7500") | ||
| .value_name("SECS") | ||
| .help("Client Config. Seconds to run benchmark, then exit"), | ||
| ) | ||
| .arg( | ||
| Arg::with_name("client_wait_for_n_nodes") | ||
| .long("client-wait-for-n-nodes") | ||
| .short('N') | ||
| .takes_value(true) | ||
| .value_name("NUM") | ||
| .help("Client Config. Optional: Wait for NUM nodes to converge"), | ||
| .help("Seconds to run benchmark, then exit"), | ||
| ) |
There was a problem hiding this comment.
Is this arg used if you're not using bench-tps or generic-client? If not, it might be clearer to keep it under the subcommands, even if it's repeated.
gregcusack
force-pushed
the
generic-client-v3
branch
from
d0a089c to
2aa6101
Compare
joncinque
approved these changes
Aug 6, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Bring your own client
Requested feature from Brennan
Bench-tps client is currently the only client you can run in validator lab. This PR allows you to deploy any client.
I took the approach of requiring the user to containerize their own client. It decouples the specific client logic with the validator lab logic.
I am not 100% sold on how this is implemented. For example, user has to build their own container image for their client and any accounts you need have to be built directly into the container image. Also, if you need cluster specific args, you'll have to modify the validator-lab code to add those cluster specific values as environment variables. For example, see
SHRED_VERSIONexample in the READMEBut figured I'd open a PR in case you all had thoughts.
(also i changed
ValidatorTypetoNodeTypesinceClientis a valid type but it is not a validator)Next PR would be to deploy just clients. So you could spin up say 20 clients that all target testnet for example.