-
Notifications
You must be signed in to change notification settings - Fork 3.9k
[CLI] Fix faucet component of local testnet without --force-restart #10214
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,7 +9,7 @@ All notable changes to the Aptos CLI will be captured in this file. This project | |
|
|
||
| ### Updated | ||
| - The `--with-faucet` flag has been removed from `aptos node run-local-testnet`, we now run a faucet by default. To disable the faucet use the `--no-faucet` flag. | ||
| - When using `aptos node run-local-testnet` we now expose a transaction stream. Learn more about the transaction stream service here: https://aptos.dev/indexer/txn-stream/. Opt out of this with `--no-txn-stream`. | ||
| - **Breaking change**: When using `aptos node run-local-testnet` we now expose a transaction stream. Learn more about the transaction stream service here: https://aptos.dev/indexer/txn-stream/. Opt out of this with `--no-txn-stream`. This is marked as a breaking change since the CLI now uses a port (50051 by default) that it didn't used to. If you need this port, you can tell the CLI to use a different port with `--txn-stream-port`. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. New port is being used now. Does this break our CLI e2e? (Can't remember if we hard coded the port in e2e)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is an additional new port, all the existing stuff is on the same port. |
||
|
|
||
| ## [2.1.0] - 2023/08/24 | ||
| ### Updated | ||
|
|
||
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.
Trying wrap my head around this...
Not sure if I read the code right, but If both
config_pathandtest_dirare provided, the config fromtest_diris ignored? Should we use the one fromtest_dirif provided?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.
Actually the other way around, if
test_diris provided thenconfig_pathis ignored. This is a pretty awful function, I'm just trying to document it, an enum would be better here.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.
I see! Yeah probably a good
ops weekitem in Q4. Lets get this fix and do a release soon to unblock 😃nit: Intuitively, I feel like if
test_diris provided, I would assumetest_diris used and overrides everything else, includingconfigThere 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.
Yeah I went back and forth on that. In reality this is just not good code and Rust has enums to solve this problem.