fix: Store machine id on connection entries so it can be removed with machine #182
Conversation
psviderski
left a comment
There was a problem hiding this comment.
Looks good! Just one comment about updating the handling of the context option.
cmd/uncloud/machine/rm.go
Outdated
| // the machine with its connection in the config, e.g. by storing the machine name in the connection metadata. | ||
| // Remove the connection to the machine from the uncloud config if it exists. | ||
| if uncli.Config != nil { | ||
| contextName := opts.context |
There was a problem hiding this comment.
I think this should be updated after #174 has been merged as the --context is now a global flag/option.
There was a problem hiding this comment.
Updated. I added a helper function to the cli with that bit of "override or current" logic since it was used in two places now:
func (cli *CLI) GetContextOverrideOrCurrent() string {
contextName := cli.contextOverride
if contextName == "" {
contextName = cli.Config.CurrentContext
}
return contextName
}
The cli's ConnectClusterWithOptions also has something similar, but it's mixed in with validation of the name and error logging. It might make sense to refactor that more later, but that would be more involved and seemed better to leave for a separate PR. These other uses have (simpler) error handling of their own already, too.
… machine rm` to clean up connections
525906d to
2f677ec
Compare
Spun out of #162 to associate machine id with corresponding connection entry.