Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(cmd): apply env vars consistently across cmd #16225

Merged
merged 1 commit into from
Dec 17, 2019
Merged

feat(cmd): apply env vars consistently across cmd #16225

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
### Features

### Bug Fixes
1. [16225](https://github.com/influxdata/influxdb/pull/16225): Ensures env vars are applied consistently across cmd, and fixes issue where INFLUX_ env var prefix was not set globally.

1. [16235](https://github.com/influxdata/influxdb/pull/16235): Removed default frontend sorting when flux queries specify sorting
1. [16238](https://github.com/influxdata/influxdb/pull/16238): Store canceled task runs in the correct bucket
Expand Down
14 changes: 14 additions & 0 deletions cmd/influx/authorization.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/influxdata/influxdb/cmd/influx/internal"
"github.com/influxdata/influxdb/http"
"github.com/spf13/cobra"
"github.com/spf13/viper"
)

// AuthorizationCreateFlags are command line args used when creating a authorization
Expand Down Expand Up @@ -77,6 +78,10 @@ func authCreateCmd() *cobra.Command {

cmd.Flags().StringVarP(&authCreateFlags.org, "org", "o", "", "The organization name (required)")
cmd.MarkFlagRequired("org")
Copy link
Contributor

Choose a reason for hiding this comment

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

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 🤷‍♂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sounds good.

viper.BindEnv("ORG")
if h := viper.GetString("ORG"); h != "" {
authCreateFlags.org = h
Copy link
Contributor

Choose a reason for hiding this comment

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

this feels off to me, I would suggest flags take precendence over env vars personally

}

cmd.Flags().StringVarP(&authCreateFlags.user, "user", "u", "", "The user name")

Expand Down Expand Up @@ -297,7 +302,16 @@ func authFindCmd() *cobra.Command {
cmd.Flags().StringVarP(&authorizationFindFlags.user, "user", "u", "", "The user")
cmd.Flags().StringVarP(&authorizationFindFlags.userID, "user-id", "", "", "The user ID")
cmd.Flags().StringVarP(&authorizationFindFlags.org, "org", "o", "", "The org")
viper.BindEnv("ORG")
if h := viper.GetString("ORG"); h != "" {
authorizationFindFlags.org = h
}

cmd.Flags().StringVarP(&authorizationFindFlags.orgID, "org-id", "", "", "The org ID")
viper.BindEnv("ORG_ID")
if h := viper.GetString("ORG_ID"); h != "" {
authorizationFindFlags.orgID = h
}
cmd.Flags().StringVarP(&authorizationFindFlags.id, "id", "i", "", "The authorization ID")

return cmd
Expand Down
46 changes: 25 additions & 21 deletions cmd/influx/bucket.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/influxdata/influxdb/cmd/influx/internal"
"github.com/influxdata/influxdb/http"
"github.com/spf13/cobra"
"github.com/spf13/viper"
)

// Bucket Command
Expand All @@ -25,9 +26,8 @@ func bucketF(cmd *cobra.Command, args []string) {

// BucketCreateFlags define the Create Command
type BucketCreateFlags struct {
name string
orgID string
org string
name string
organization
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd recommend not embedding the organization type, although, all this will likely get torched when a testable design comes in 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

totally agree, I wanted this to be easy to revert once we converge on a pattern we're implementing across the board.

retention time.Duration
}

Expand All @@ -42,9 +42,8 @@ func init() {

bucketCreateCmd.Flags().StringVarP(&bucketCreateFlags.name, "name", "n", "", "Name of bucket that will be created")
bucketCreateCmd.Flags().DurationVarP(&bucketCreateFlags.retention, "retention", "r", 0, "Duration in nanoseconds data will live in bucket")
bucketCreateCmd.Flags().StringVarP(&bucketCreateFlags.orgID, "org-id", "", "", "The ID of the organization that owns the bucket")
bucketCreateCmd.Flags().StringVarP(&bucketCreateFlags.org, "org", "o", "", "The org name")
bucketCreateCmd.MarkFlagRequired("name")
bucketCreateFlags.organization.register(bucketCreateCmd)

bucketCmd.AddCommand(bucketCreateCmd)
}
Expand All @@ -65,10 +64,8 @@ func newBucketService(f Flags) (platform.BucketService, error) {
}

func bucketCreateF(cmd *cobra.Command, args []string) error {
if bucketCreateFlags.orgID == "" && bucketCreateFlags.org == "" {
return fmt.Errorf("must specify org-id, or org name")
} else if bucketCreateFlags.orgID != "" && bucketCreateFlags.org != "" {
return fmt.Errorf("must specify org-id, or org name not both")
if err := bucketCreateFlags.organization.validOrgFlags(); err != nil {
return err
}

s, err := newBucketService(flags)
Expand All @@ -86,7 +83,7 @@ func bucketCreateF(cmd *cobra.Command, args []string) error {
return nil
}

b.OrgID, err = getOrgID(orgSvc, bucketCreateFlags.orgID, bucketCreateFlags.org)
b.OrgID, err = bucketCreateFlags.organization.getID(orgSvc)
if err != nil {
return err
}
Expand Down Expand Up @@ -117,9 +114,8 @@ func bucketCreateF(cmd *cobra.Command, args []string) error {
type BucketFindFlags struct {
name string
id string
org string
orgID string
headers bool
organization
}

var bucketFindFlags BucketFindFlags
Expand All @@ -132,10 +128,13 @@ func init() {
}

bucketFindCmd.Flags().StringVarP(&bucketFindFlags.name, "name", "n", "", "The bucket name")
viper.BindEnv("BUCKET_NAME")
if h := viper.GetString("BUCKET_NAME"); h != "" {
bucketFindFlags.name = h
}
bucketFindCmd.Flags().StringVarP(&bucketFindFlags.id, "id", "i", "", "The bucket ID")
bucketFindCmd.Flags().StringVarP(&bucketFindFlags.orgID, "org-id", "", "", "The bucket organization ID")
bucketFindCmd.Flags().StringVarP(&bucketFindFlags.org, "org", "o", "", "The bucket organization name")
bucketFindCmd.Flags().BoolVar(&bucketFindFlags.headers, "headers", true, "To print the table headers; defaults true")
bucketFindFlags.organization.register(bucketFindCmd)

bucketCmd.AddCommand(bucketFindCmd)
}
Expand All @@ -159,20 +158,20 @@ func bucketFindF(cmd *cobra.Command, args []string) error {
filter.ID = id
}

if bucketFindFlags.orgID != "" && bucketFindFlags.org != "" {
return fmt.Errorf("must specify at exactly one of org and org-id")
if err := bucketFindFlags.organization.validOrgFlags(); err != nil {
return err
}

if bucketFindFlags.orgID != "" {
orgID, err := platform.IDFromString(bucketFindFlags.orgID)
if bucketFindFlags.organization.id != "" {
orgID, err := platform.IDFromString(bucketFindFlags.organization.id)
if err != nil {
return fmt.Errorf("failed to decode org id %q: %v", bucketFindFlags.orgID, err)
return fmt.Errorf("failed to decode org id %q: %v", bucketFindFlags.organization.id, err)
}
filter.OrganizationID = orgID
}

if bucketFindFlags.org != "" {
filter.Org = &bucketFindFlags.org
if bucketFindFlags.organization.name != "" {
filter.Org = &bucketFindFlags.organization.name
}

buckets, _, err := s.FindBuckets(context.Background(), filter)
Expand Down Expand Up @@ -219,6 +218,11 @@ func init() {

bucketUpdateCmd.Flags().StringVarP(&bucketUpdateFlags.id, "id", "i", "", "The bucket ID (required)")
bucketUpdateCmd.Flags().StringVarP(&bucketUpdateFlags.name, "name", "n", "", "New bucket name")
viper.BindEnv("BUCKET_NAME")
if h := viper.GetString("BUCKET_NAME"); h != "" {
bucketFindFlags.name = h
}

bucketUpdateCmd.Flags().DurationVarP(&bucketUpdateFlags.retention, "retention", "r", 0, "New duration data will live in bucket")
bucketUpdateCmd.MarkFlagRequired("id")

Expand Down
19 changes: 8 additions & 11 deletions cmd/influx/inspect.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ type InspectReportTSMFlags struct {
pattern string
exact bool
detailed bool

orgID, org, bucketID string
dataDir string
organization
bucketID string
dataDir string
}

var inspectReportTSMFlags InspectReportTSMFlags
Expand Down Expand Up @@ -62,8 +62,7 @@ in the following ways:
inspectReportTSMCommand.Flags().BoolVarP(&inspectReportTSMFlags.exact, "exact", "", false, "calculate and exact cardinality count. Warning, may use significant memory...")
inspectReportTSMCommand.Flags().BoolVarP(&inspectReportTSMFlags.detailed, "detailed", "", false, "emit series cardinality segmented by measurements, tag keys and fields. Warning, may take a while.")

inspectReportTSMCommand.Flags().StringVarP(&inspectReportTSMFlags.orgID, "org-id", "", "", "process only data belonging to organization ID.")
inspectReportTSMCommand.Flags().StringVarP(&inspectReportTSMFlags.org, "org", "o", "", "process only data belonging to organization name.")
inspectReportTSMFlags.organization.register(inspectReportTSMCommand)
inspectReportTSMCommand.Flags().StringVarP(&inspectReportTSMFlags.bucketID, "bucket-id", "", "", "process only data belonging to bucket ID. Requires org flag to be set.")

dir, err := fs.InfluxDir()
Expand All @@ -76,10 +75,8 @@ in the following ways:

// inspectReportTSMF runs the report-tsm tool.
func inspectReportTSMF(cmd *cobra.Command, args []string) error {
if inspectReportTSMFlags.orgID == "" && inspectReportTSMFlags.org == "" {
return fmt.Errorf("must specify org-id, or org name")
} else if inspectReportTSMFlags.orgID != "" && inspectReportTSMFlags.org != "" {
return fmt.Errorf("must specify org-id, or org name not both")
if err := inspectReportTSMFlags.organization.validOrgFlags(); err != nil {
return err
}
report := &tsm1.Report{
Stderr: os.Stderr,
Expand All @@ -90,15 +87,15 @@ func inspectReportTSMF(cmd *cobra.Command, args []string) error {
Exact: inspectReportTSMFlags.exact,
}

if (inspectReportTSMFlags.org == "" || inspectReportTSMFlags.orgID == "") && inspectReportTSMFlags.bucketID != "" {
if (inspectReportTSMFlags.organization.name == "" || inspectReportTSMFlags.organization.id == "") && inspectReportTSMFlags.bucketID != "" {
return errors.New("org-id must be set for non-empty bucket-id")
}

orgSvc, err := newOrganizationService()
if err != nil {
return nil
}
id, err := getOrgID(orgSvc, bucketCreateFlags.orgID, bucketCreateFlags.org)
id, err := inspectReportTSMFlags.organization.getID(orgSvc)
if err != nil {
return nil
}
Expand Down
42 changes: 34 additions & 8 deletions cmd/influx/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ func influxCmd() *cobra.Command {
cmd.Usage()
},
}

viper.SetEnvPrefix("INFLUX")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

caught this when testing the cmd after these changes - we should've been calling this before adding any of the commands below.


cmd.AddCommand(
authCmd(),
bucketCmd,
Expand All @@ -99,8 +102,6 @@ func influxCmd() *cobra.Command {
writeCmd,
)

viper.SetEnvPrefix("INFLUX")

cmd.PersistentFlags().StringVarP(&flags.token, "token", "t", "", "API token to be used throughout client calls")
viper.BindEnv("TOKEN")
if h := viper.GetString("TOKEN"); h != "" {
Expand Down Expand Up @@ -230,22 +231,47 @@ func newLocalKVService() (*kv.Service, error) {
return kv.NewService(zap.NewNop(), store), nil
}

func getOrgID(orgSVC influxdb.OrganizationService, id string, name string) (influxdb.ID, error) {
if id != "" {
influxOrgID, err := influxdb.IDFromString(id)
type organization struct {
id, name string
}

func (org *organization) register(cmd *cobra.Command) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this looks much improved 👍

cmd.Flags().StringVarP(&org.id, "org-id", "", "", "The ID of the organization that owns the bucket")
viper.BindEnv("ORG_ID")
if h := viper.GetString("ORG_ID"); h != "" {
org.id = h
}
cmd.Flags().StringVarP(&org.name, "org", "o", "", "The name of the organization that owns the bucket")
viper.BindEnv("ORG")
if h := viper.GetString("ORG"); h != "" {
org.name = h
}
}

func (org *organization) getID(orgSVC influxdb.OrganizationService) (influxdb.ID, error) {
if org.id != "" {
influxOrgID, err := influxdb.IDFromString(org.id)
if err != nil {
return 0, fmt.Errorf("invalid org ID provided: %s", err.Error())
}
return *influxOrgID, nil
} else if name != "" {
} else if org.name != "" {
org, err := orgSVC.FindOrganization(context.Background(), influxdb.OrganizationFilter{
Name: &name,
Name: &org.name,
})
if err != nil {
return 0, fmt.Errorf("%v", err)
}
return org.ID, nil
}
return 0, fmt.Errorf("failed to locate an organization id")
}

return 0, fmt.Errorf("")
func (org *organization) validOrgFlags() error {
if org.id == "" && org.name == "" {
return fmt.Errorf("must specify org-id, or org name")
} else if org.id != "" && org.name != "" {
return fmt.Errorf("must specify org-id, or org name not both")
}
return nil
}
49 changes: 48 additions & 1 deletion cmd/influx/organization.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/influxdata/influxdb/cmd/influx/internal"
"github.com/influxdata/influxdb/http"
"github.com/spf13/cobra"
"github.com/spf13/viper"
)

func organizationCmd() *cobra.Command {
Expand Down Expand Up @@ -113,7 +114,15 @@ func orgFindCmd() *cobra.Command {
}

cmd.Flags().StringVarP(&organizationFindFlags.name, "name", "n", "", "The organization name")
viper.BindEnv("ORG")
if h := viper.GetString("ORG"); h != "" {
organizationFindFlags.name = h
}
cmd.Flags().StringVarP(&organizationFindFlags.id, "id", "i", "", "The organization ID")
viper.BindEnv("ORG_ID")
if h := viper.GetString("ORG_ID"); h != "" {
organizationFindFlags.id = h
}

return cmd
}
Expand Down Expand Up @@ -174,8 +183,17 @@ func orgUpdateCmd() *cobra.Command {
}

cmd.Flags().StringVarP(&organizationUpdateFlags.id, "id", "i", "", "The organization ID (required)")
cmd.Flags().StringVarP(&organizationUpdateFlags.name, "name", "n", "", "The organization name")
cmd.MarkFlagRequired("id")
viper.BindEnv("ORG_ID")
if h := viper.GetString("ORG_ID"); h != "" {
organizationUpdateFlags.id = h
}

cmd.Flags().StringVarP(&organizationUpdateFlags.name, "name", "n", "", "The organization name")
viper.BindEnv("ORG")
if h := viper.GetString("ORG"); h != "" {
organizationUpdateFlags.name = h
}

return cmd
}
Expand Down Expand Up @@ -268,6 +286,10 @@ func orgDeleteCmd() *cobra.Command {

cmd.Flags().StringVarP(&organizationDeleteFlags.id, "id", "i", "", "The organization ID (required)")
cmd.MarkFlagRequired("id")
viper.BindEnv("ORG_ID")
if h := viper.GetString("ORG_ID"); h != "" {
organizationUpdateFlags.id = h
}

return cmd
}
Expand Down Expand Up @@ -340,7 +362,15 @@ func orgMembersListCmd() *cobra.Command {
}

cmd.Flags().StringVarP(&organizationMembersListFlags.id, "id", "i", "", "The organization ID")
viper.BindEnv("ORG_ID")
if h := viper.GetString("ORG_ID"); h != "" {
organizationMembersListFlags.id = h
}
cmd.Flags().StringVarP(&organizationMembersListFlags.name, "name", "n", "", "The organization name")
viper.BindEnv("ORG")
if h := viper.GetString("ORG"); h != "" {
organizationMembersListFlags.name = h
}

return cmd
}
Expand Down Expand Up @@ -411,7 +441,16 @@ func orgMembersAddCmd() *cobra.Command {
}

cmd.Flags().StringVarP(&organizationMembersAddFlags.id, "id", "i", "", "The organization ID")
viper.BindEnv("ORG_ID")
if h := viper.GetString("ORG_ID"); h != "" {
organizationMembersAddFlags.id = h
}
cmd.Flags().StringVarP(&organizationMembersAddFlags.name, "name", "n", "", "The organization name")
viper.BindEnv("ORG")
if h := viper.GetString("ORG"); h != "" {
organizationMembersAddFlags.name = h
}

cmd.Flags().StringVarP(&organizationMembersAddFlags.memberID, "member", "o", "", "The member ID")
cmd.MarkFlagRequired("member")

Expand Down Expand Up @@ -478,7 +517,15 @@ func orgMembersRemoveCmd() *cobra.Command {
}

cmd.Flags().StringVarP(&organizationMembersRemoveFlags.id, "id", "i", "", "The organization ID")
viper.BindEnv("ORG_ID")
if h := viper.GetString("ORG_ID"); h != "" {
organizationMembersAddFlags.id = h
}
cmd.Flags().StringVarP(&organizationMembersRemoveFlags.name, "name", "n", "", "The organization name")
viper.BindEnv("ORG")
if h := viper.GetString("ORG"); h != "" {
organizationMembersRemoveFlags.name = h
}
cmd.Flags().StringVarP(&organizationMembersRemoveFlags.memberID, "member", "o", "", "The member ID")
cmd.MarkFlagRequired("member")

Expand Down
Loading