Skip to content
This repository was archived by the owner on Jun 13, 2025. It is now read-only.

[cluster] Add selected cluster to cluster provider interface#13

Merged
mikeland73 merged 1 commit intomainfrom
landau/remove-cluster-name-from-cluster-get
Nov 8, 2022
Merged

[cluster] Add selected cluster to cluster provider interface#13
mikeland73 merged 1 commit intomainfrom
landau/remove-cluster-name-from-cluster-get

Conversation

@mikeland73
Copy link
Copy Markdown
Contributor

Summary

This simplifies the cluster provider interface by removing the need to pass a cluster name every time we call Get(). This uses state in the provider which is associated to the cluster flag and also set when the jetconfig is read.

How was it tested?

  • Compiles/tests
  • launchpad up (on commercial version) (with jetconfig cluster and also using --cluster flag)

Is this change backwards-compatible?

yes

@mikeland73 mikeland73 requested review from LucilleH and ipince November 8, 2022 04:37
@mikeland73 mikeland73 changed the title [cluster] Add selected cluster to interface [cluster] Add selected cluster to cluster provider interface Nov 8, 2022
Comment thread padcli/command/config.go
}

if *providers.ClusterProvider().GetSelectedClusterName() == "" {
providers.ClusterProvider().SetSelectedClusterName(c.Cluster)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still care about the --cluster flag?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want we can remove, but I didn't want to make that call in this PR. FWIW, I find it useful for testing (e.g. -c docker-desktop)

@mikeland73 mikeland73 merged commit e981bc1 into main Nov 8, 2022
@mikeland73 mikeland73 deleted the landau/remove-cluster-name-from-cluster-get branch November 8, 2022 04:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants