feat: handle pagination in bucket list (#307)

This commit is contained in:
Daniel Moran 2021-10-15 15:14:59 -07:00 committed by GitHub
parent d92bede8d3
commit 6fda4cceed
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 165 additions and 7 deletions

View File

@ -4,6 +4,7 @@
1. [259](https://github.com/influxdata/influx-cli/pull/259): Add `-b` shorthand for `--bucket` to `delete`
1. [285](https://github.com/influxdata/influx-cli/pull/285): Add short-hand `--all-access` and `--operator` options to `auth create`.
1. [307](https://github.com/influxdata/influx-cli/pull/307): Handle pagination in `bucket list` to support displaying more than 20 buckets.
### Bug Fixes

View File

@ -4,14 +4,18 @@ import (
"context"
"fmt"
"github.com/influxdata/influx-cli/v2/api"
"github.com/influxdata/influx-cli/v2/clients"
)
type BucketsListParams struct {
OrgID string
OrgName string
Name string
ID string
OrgID string
OrgName string
Name string
ID string
Limit int
Offset int
PageSize int
}
func (c Client) List(ctx context.Context, params *BucketsListParams) error {
@ -29,16 +33,95 @@ func (c Client) List(ctx context.Context, params *BucketsListParams) error {
if params.OrgID == "" && params.OrgName == "" {
req = req.Org(c.ActiveConfig.Org)
}
if params.Name != "" {
req = req.Name(params.Name)
if params.Name != "" || params.ID != "" {
return c.findOneBucket(params, req)
}
// Set the limit for the number of items to return per HTTP request.
// NOTE this is not the same as the `--limit` option to the CLI, which sets
// the max number of rows to pull across _all_ HTTP requests in total.
limit := params.PageSize
// Adjust if the total limit < the per-request limit.
// This is convenient for users, since the per-request limit has a default
// value that people might not want to override on every CLI call.
if params.Limit != 0 && params.Limit < limit {
limit = params.Limit
}
offset := params.Offset
printOpts := bucketPrintOptions{}
// Iteratively fetch pages of bucket metadata.
//
// NOTE: This algorithm used an `offset`-based pagination. The API also documents that an `after`-based
// approach would work. Pagination via `after` would typically be much more efficient than the `offset`-
// based approach used here, but we still use `offset` because:
// 1. As of this writing (10/15/2021) Cloud doesn't support pagination via `after` when filtering by org.
// Versions of OSS prior to v2.1 had the same restriction. Both Cloud and OSS have supported `offset`-
// based pagination since before the OSS/Cloud split.
// 2. Listing buckets-by-org uses a secondary index on orgID+bucketName in both OSS and Cloud. Since bucket
// ID isn't part of the index, implementing pagination via `after` requires scanning the subset of the
// index for the target org until the given ID is found. This is just as (in)efficient as the implementation
// of `offset`-based filtering.
//
// If you are thinking of copy-paste-adjusting this code for another command, CONSIDER USING `after` INSTEAD OF
// `offset`! The conditions that make `offset` OK for buckets probably aren't present for other resource types.
//
// We should eventually convert this to use `after` for consistency across the CLI.
for {
req := req
if limit != 0 {
req = req.Limit(int32(limit))
}
if offset != 0 {
req = req.Offset(int32(offset))
}
res, err := req.Execute()
if err != nil {
return fmt.Errorf("failed to list buckets: %w", err)
}
var buckets []api.Bucket
if res.Buckets != nil {
buckets = *res.Buckets
}
printOpts.buckets = append(printOpts.buckets, buckets...)
// Break out of the loop if:
// * The server returned less than a full page of results
// * We've collected as many results as the user asked for
if len(buckets) == 0 || len(buckets) < params.PageSize ||
(params.Limit != 0 && len(printOpts.buckets) == params.Limit) {
break
}
// Adjust the page-size for the next request (if needed) so we don't pull down more information
// than the user requested.
if params.Limit != 0 && len(printOpts.buckets)+limit > params.Limit {
limit = params.Limit - len(printOpts.buckets)
}
// Bump the offset for the next request to pull the next page.
offset = offset + len(buckets)
}
return c.printBuckets(printOpts)
}
// findOneBucket queries for a single bucket resoruce using the list API.
// Used to look up buckets by ID or name.
func (c Client) findOneBucket(params *BucketsListParams, req api.ApiGetBucketsRequest) error {
var description string
if params.ID != "" {
req = req.Id(params.ID)
description = " by ID"
} else if params.Name != "" {
req = req.Name(params.Name)
description = " by name"
}
buckets, err := req.Execute()
if err != nil {
return fmt.Errorf("failed to list buckets: %w", err)
return fmt.Errorf("failed to find bucket%s: %w", description, err)
}
printOpts := bucketPrintOptions{}

View File

@ -89,6 +89,62 @@ func TestBucketsList(t *testing.T) {
`123\s+my-bucket\s+1h0m0s\s+n/a\s+456`,
},
},
{
name: "pagination via 'offset'",
params: bucket.BucketsListParams{
PageSize: 2,
Offset: 1,
},
configOrgName: "my-default-org",
registerBucketExpectations: func(t *testing.T, bucketsApi *mock.MockBucketsApi) {
bucketsApi.EXPECT().GetBuckets(gomock.Any()).Return(api.ApiGetBucketsRequest{ApiService: bucketsApi})
bucketsApi.EXPECT().GetBucketsExecute(tmock.MatchedBy(func(in api.ApiGetBucketsRequest) bool {
return assert.Equal(t, "my-default-org", *in.GetOrg()) &&
assert.Equal(t, int32(2), *in.GetLimit()) &&
assert.Equal(t, int32(1), *in.GetOffset())
})).Return(api.Buckets{
Buckets: &[]api.Bucket{
{
Id: api.PtrString("222"),
Name: "my-bucket2",
OrgID: api.PtrString("456"),
RetentionRules: []api.RetentionRule{
{EverySeconds: 2400},
},
},
{
Id: api.PtrString("333"),
Name: "my-bucket3",
OrgID: api.PtrString("456"),
RetentionRules: []api.RetentionRule{
{EverySeconds: 3600},
},
},
},
}, nil)
bucketsApi.EXPECT().GetBucketsExecute(tmock.MatchedBy(func(in api.ApiGetBucketsRequest) bool {
return assert.Equal(t, "my-default-org", *in.GetOrg()) &&
assert.Equal(t, int32(2), *in.GetLimit()) &&
assert.Equal(t, int32(3), *in.GetOffset())
})).Return(api.Buckets{
Buckets: &[]api.Bucket{
{
Id: api.PtrString("444"),
Name: "my-bucket4",
OrgID: api.PtrString("456"),
RetentionRules: []api.RetentionRule{
{EverySeconds: 4800},
},
},
},
}, nil)
},
expectedStdoutPatterns: []string{
`222\s+my-bucket2\s+40m0s\s+n/a\s+456`,
`333\s+my-bucket3\s+1h0m0s\s+n/a\s+456`,
`444\s+my-bucket4\s+1h20m0s\s+n/a\s+456`,
},
},
{
name: "override org by ID",
params: bucket.BucketsListParams{
@ -109,6 +165,7 @@ func TestBucketsList(t *testing.T) {
name: "override org by name",
params: bucket.BucketsListParams{
OrgName: "my-org",
Limit: 2,
},
configOrgName: "my-default-org",
registerBucketExpectations: func(t *testing.T, bucketsApi *mock.MockBucketsApi) {
@ -148,6 +205,7 @@ func TestBucketsList(t *testing.T) {
name: "list multiple bucket schema types",
params: bucket.BucketsListParams{
OrgName: "my-org",
Limit: 3,
},
configOrgName: "my-default-org",
registerBucketExpectations: func(t *testing.T, bucketsApi *mock.MockBucketsApi) {

View File

@ -156,6 +156,22 @@ func newBucketListCmd() cli.Command {
EnvVar: "INFLUX_ORG",
Destination: &params.OrgName,
},
&cli.IntFlag{
Name: "limit",
Usage: "Total number of buckets to fetch from the server, or 0 to return all buckets",
Destination: &params.Limit,
},
&cli.IntFlag{
Name: "offset",
Usage: "Number of buckets to skip over in the list",
Destination: &params.Offset,
},
&cli.IntFlag{
Name: "page-size",
Usage: "Number of buckets to fetch per request to the server",
Value: 20,
Destination: &params.PageSize,
},
),
Action: func(ctx *cli.Context) error {
api := getAPI(ctx)