Conversation
|
It solves #2688, does this mean we can have two separate active clusters of local machine validator now at the same time? |
|
I think it does since it also solves #2528 |
Both issues are pretty similar. Answer is yes we can have two separate local clusters at the same time. |
| string, // nodeID | ||
| string, // public key | ||
| string, // PoP |
There was a problem hiding this comment.
should we just return a nodeID object here, instead of three strings?
There was a problem hiding this comment.
I prefer this reformatting to be done on another PR
pkg/localnet/localnet.go
Outdated
| } else if !hasValidators { | ||
| continue | ||
| } | ||
| blockchainBootstrapped, err := IsLocalClusterBlockchainBootstrapped(app, clusterName, blockchain.ID.String(), blockchain.SubnetID) |
There was a problem hiding this comment.
why not just assign blockchainBootstrappedOnSomeCluster here
There was a problem hiding this comment.
we iterate over all the clusters , the last one can be false and overwrite some previous true value. I believe it is cleaner this way that to assign to blockchainBootstrappedOnSomeCluster, check the value and break if true
There was a problem hiding this comment.
ok will implement the version with break
| func GetFilteredLocalClusters( | ||
| app *application.Avalanche, | ||
| running bool, | ||
| network models.Network, | ||
| blockchainName string, | ||
| ) ([]string, error) { |
There was a problem hiding this comment.
Seems that this function is used whenever running = true, should we remove this from argument?
There was a problem hiding this comment.
I would rather prefer to keep the option available for the future. It is useful for debugging and dev tasks.
Seems that node local destroy doesn't output anything. |
After i upgrade to a new cli release using this PR, If i want to deploy a new local network, seems that I have to wait a while for migration if i have any local nodes left (which people will do if they dont call node local destroy), can we give them an option if they want to migrate? If they dont' want to migrate, we will just destroy these local nodes instead of migrating them |
Why this should be merged
Closes #2579 #2091 #2528 #2688
There is also a couple of minor extra features added on it:
network statusoutputHow this works
How this was tested
How is this documented