feat(cmd): apply env vars consistently across cmd #16225
Conversation
dearyhud
force-pushed
the
16214/consistently-apply-env-vars
branch
3 times, most recently
from
03b265b
to
6b79682
Compare
| @@ -83,6 +83,9 @@ func influxCmd() *cobra.Command { | |||
| cmd.Usage() | |||
| }, | |||
| } | |||
|
|
|||
| viper.SetEnvPrefix("INFLUX") | |||
There was a problem hiding this comment.
caught this when testing the cmd after these changes - we should've been calling this before adding any of the commands below.
dearyhud
force-pushed
the
16214/consistently-apply-env-vars
branch
from
6b79682
to
9aa343a
Compare
| @@ -77,6 +78,10 @@ func authCreateCmd() *cobra.Command { | |||
|
|
|||
| cmd.Flags().StringVarP(&authCreateFlags.org, "org", "o", "", "The organization name (required)") | |||
| cmd.MarkFlagRequired("org") | |||
| viper.BindEnv("ORG") | |||
| if h := viper.GetString("ORG"); h != "" { | |||
| authCreateFlags.org = h | |||
There was a problem hiding this comment.
this feels off to me, I would suggest flags take precendence over env vars personally
| @@ -77,6 +78,10 @@ func authCreateCmd() *cobra.Command { | |||
|
|
|||
| cmd.Flags().StringVarP(&authCreateFlags.org, "org", "o", "", "The organization name (required)") | |||
| cmd.MarkFlagRequired("org") | |||
There was a problem hiding this comment.
can you make this into a reusable type? somethign like
// rename I don't know what to call it 😬
type orgStuffs struct {
orgID string
organization string
}
func (o *orgStuffs) register(cmd *cobra.Cmd) {
viper.BindEnv("ORG")
cmd.Flags().StringVarp(&o.organization,"org", "o", "", "The organization name (required)")
... viper stuffs
... wash rise repeat for org-id in here
}then just make that a dep of the builders or inline it wherever, a step towards reusable patterns for influx cli tool 🤷♂
dearyhud
force-pushed
the
16214/consistently-apply-env-vars
branch
4 times, most recently
from
8bae028
to
116f90b
Compare
dearyhud
force-pushed
the
16214/consistently-apply-env-vars
branch
from
116f90b
to
6dc9f14
Compare
| orgID string | ||
| org string | ||
| name string | ||
| organization |
There was a problem hiding this comment.
I'd recommend not embedding the organization type, although, all this will likely get torched when a testable design comes in 🤔
There was a problem hiding this comment.
totally agree, I wanted this to be easy to revert once we converge on a pattern we're implementing across the board.
cmd/influx/bucket.go
Outdated
| return fmt.Errorf("must specify org-id, or org name") | ||
| } else if bucketCreateFlags.orgID != "" && bucketCreateFlags.org != "" { | ||
| } else if bucketCreateFlags.organization.id != "" && bucketCreateFlags.organization.name != "" { |
There was a problem hiding this comment.
you can make this a method on the organization type itself and call it here 👍
There was a problem hiding this comment.
ah - I did https://github.com/influxdata/influxdb/pull/16225/files#diff-0f706fc08b27c10d38fa0be84c6ebf3eR270 just forgot to call it. good catch!
| id, name string | ||
| } | ||
|
|
||
| func (org *organization) register(cmd *cobra.Command) { |
There was a problem hiding this comment.
this looks much improved 👍
cmd/influx/pkg.go
Outdated
| orgID string | ||
| org string | ||
| quiet bool | ||
| organization |
There was a problem hiding this comment.
lets not embed this here, namespace it by a field name, something as simple as the following would be fine.
org organization
dearyhud
force-pushed
the
16214/consistently-apply-env-vars
branch
from
6dc9f14
to
a38d2fd
Compare
dearyhud
force-pushed
the
16214/consistently-apply-env-vars
branch
from
a38d2fd
to
e959a63
Compare
Closes #16214
Consistently applies env vars to cli commands using flags that could be interchanged with env vars (ie. --org-id INFLUX_ORG_ID, --bucket-name INFLUX_BUCKET_NAME, etc). See #16048 for a more detailed list of env vars.
Also fixes a small bug where we were not setting globally the
INFLUXprefix before any cobra commands were init.