-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Support ad-hoc genesis args in run.sh #9697
Conversation
run.sh
Outdated
@@ -97,7 +99,8 @@ args=( | |||
--enable-rpc-transaction-history | |||
--init-complete-file "$dataDir"/init-completed | |||
) | |||
solana-validator "${args[@]}" & | |||
# shellcheck disable=SC2086 | |||
solana-validator "${args[@]}" $SOLANA_RUN_SH_VALIDATOR_ARGS & |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know if we have a user for ${args[@]}
? Just seems a little complex to have both args coming in via arguments and environment variables. I wonder if we should do something like:
$ run.sh <genesis args> -- <validator args>
with the default of
$ run.sh <validator args>
if --
is unspecified.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, my eye was blind.. I can just use ${args[@]}
instead of another env....
IIRC, there should be little user for ${args[@]}
if ever. Yet, some validator-launching convenient scripts in example apps might be passing some custom args?
The main motivation of this PR is SOLANA_RUN_SH_GENESIS_ARGS
.
I defined SOLANA_RUN_SH_VALIDATOR_ARGS
for consistency and as a bonus. I even considered SOLANA_RUN_SH_FAUCET_ARGS
... ;)
I guess the --
escaping might be too tedious like run.sh <FAUCET ARGS> -- <GENESIS ARGS> -- <VALIDATOR ARGS>
...?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, just adding SOLANA_RUN_SH_GENESIS_ARGS seems ok for now. But also not really a big deal, do whatever works best for you :)
automerge (cherry picked from commit d44e0b7)
automerge (cherry picked from commit d44e0b7)
Some times I want to add some ad-hoc options to
solana-genesis
andsolana-validator
like--enable-warmup-epochs
.Split off from #9527.