cmd: create cluster cleanup
#2392
Conversation
- removed `--clean` subcommand in favor of explicit error messages - removed default for `--nodes` - removed default for `--network` - the default cluster dir is now the current working directory
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #2392 +/- ##
==========================================
- Coverage 53.65% 53.37% -0.29%
==========================================
Files 192 196 +4
Lines 25990 26254 +264
==========================================
+ Hits 13946 14012 +66
- Misses 10318 10505 +187
- Partials 1726 1737 +11
☔ View full report in Codecov by Sentry. |
Co-authored-by: Abhishek Kumar <43061995+xenowits@users.noreply.github.com>
cmd/createcluster.go
Outdated
| } else if _, err = os.Stat(path.Join(nodeDir(conf.ClusterDir, 0), "cluster-lock.json")); err == nil { | ||
| return errors.New("existing cluster found. Try again with --clean") | ||
|
|
||
| if conf.NumNodes == 0 { |
There was a problem hiding this comment.
we can also use wrapPreRunE function to do these config validations like we are using in other commands, WDYT?
There was a problem hiding this comment.
Mmmh I can do that, but it requires refactoring the tests quite a bit because right now we're calling the runCreateCluster straight away.
@corverroos do you think it's worth?
There was a problem hiding this comment.
Important thing is that this is tested.
Don't mind adding this in the command itself, what about extracting a verifyConfig and calling that here?
There was a problem hiding this comment.
Yeah I can extract config checks in verifyConfig.
Important thing is that this is tested.
It is, I added test cases specifically to test configuration verification.
cmd/createcluster.go
Outdated
| } else if _, err = os.Stat(path.Join(nodeDir(conf.ClusterDir, 0), "cluster-lock.json")); err == nil { | ||
| return errors.New("existing cluster found. Try again with --clean") | ||
|
|
||
| if conf.NumNodes == 0 { |
There was a problem hiding this comment.
Important thing is that this is tested.
Don't mind adding this in the command itself, what about extracting a verifyConfig and calling that here?
Co-authored-by: Abhishek Kumar <43061995+xenowits@users.noreply.github.com>
gsora
added
the
merge when ready
v0.17.0 will change how `charon create cluster` works, so update the relevant mentions here to reflect the changes. See PR ObolNetwork/charon#2392.
* update docker-compose.yml and README.md for v0.17.0 v0.17.0 will change how `charon create cluster` works, so update the relevant mentions here to reflect the changes. See PR ObolNetwork/charon#2392. * update charon version * Make new create cluster output the same as the old structure so people can continue to upgrade until we have a deprecation path for this repo * Remove most of the readme and add warnings * Missed an important '.' * Add back in sample --------- Co-authored-by: Oisín Kyne <oisin@obol.tech>
--cleansubcommand in favor of explicit error messages--nodes--networkCloses #2302.
category: refactor
ticket: #2302