refactor: Move from global state to functions #53

stuartcarnie · 2021-04-29T09:23:26Z

This PR represents a few experiments of features I've used in Cobra and some attempts to reduce duplication of code.

Uses cli.GenericFlag to encapsulate parsing and validation of flag values at parse time. This removes the burden from the individual CLI commands to parse and validate args and options.
Add flag.ID that may be used by any flag that requires an Influx ID. flag.ID parses and validates string value is a valid ID, removing this burden from individual commands and ensuring valid values before the command actions begins.
Adds a Flags() method to the Params structs to directly capture the values when parsing flags.
Moves from global state to local builder functions for the majority of the commands. This allows the commands to bind to flag variables reducing the repeated ctx.String(), ctx.Int(), etc
Leverages the BeforeFunc to create middleware and inject the CLI and API client into commands, saving the repeated boilerplate across all of the instantiated commands. This is extensible, so additional middleware can be appends using the middleware.WithBeforeFns

cmd/influx/bucket.go

cmd/influx/main.go

internal/bucket.go

internal/linereader/multi_reader.go

stuartcarnie · 2021-04-29T09:43:54Z

cmd/influx/bucket.go

+			clients := getBucketsClient(ctx)
+			return getCLI(ctx).BucketsDelete(ctx.Context, &clients, &params)


All the bucket actions are reduced to 1-2 lines of code.

cmd/influx/main.go

internal/bucket.go

internal/throttler/throttler.go

internal/write.go

internal/throttler/throttler.go

pkg/influxid/id.go

pkg/cmd/flag/bytes_per_seconds.go

cmd/influx/write.go

pkg/cli/flag/write_precision.go

pkg/cli/middleware/middleware.go

This commit represents a few experiments of features I've used in Cobra 1. Uses cli.GenericFlag to encapsulate parsing and validation of flag values at parse time. This removes the burden from the individual CLI commands to parse and validate args and options. 2. Add influxid.ID that may be used by any flag that requires an Influx ID. influxid.ID parses and validates string value is a valid ID, removing this burden from individual commands and ensuring valid values before the command actions begins. 3. Binds cli.Flags directly to params structures to directly capture the values when parsing flags. 4. Moves from global state to local builder functions for the majority of the commands. This allows the commands to bind to flag variables reducing the repeated ctx.String(), ctx.Int(), etc 5. Leverages the BeforeFunc to create middleware and inject the CLI and API client into commands, saving the repeated boilerplate across all of the instantiated commands. This is extensible, so additional middleware can be appends using the middleware.WithBeforeFns

stuartcarnie · 2021-05-03T02:51:48Z

Thanks for the review, @danxmoran. I've pushed a commit with the feedback suggestions.

danxmoran · 2021-05-03T13:30:18Z

Sorry to dirty the commit history, didn't want to rebase/force-push and disrupt your other open PR 😬

danxmoran

I'll merge when CI is green to unblock other work in the repo using the new middleware. Thanks for taking the time to contribute the improvements!

danxmoran added 9 commits April 27, 2021 17:22

feat: add /buckets APIs to codegen

1ca9b3d

feat: add /orgs API to codegen

265df6b

fix: update codegen to add id param

4d57664

fix: update codegen to pull in fixed APIs

cf130b1

feat: implement core bucket functionality

6f4f500

feat: add bucket CLI commands

bb22fed

chore: update openapi sha to point at master, fix formatting

457a98b

fix: fix bad rebase

44ebf57

fix: staticcheck

81cf5d7

stuartcarnie self-assigned this Apr 29, 2021

stuartcarnie force-pushed the sgc/experiments branch from 0d4a6dc to 4cfb5de Compare April 29, 2021 09:30