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.

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(users): list users via the API now supports offset, limit or after parameter #21367

Merged
merged 4 commits into from
May 12, 2021
Merged

feat(users): list users via the API now supports offset, limit or after parameter #21367

merged 4 commits into from
May 12, 2021

Conversation

bednar
Copy link
Contributor

@bednar bednar commented May 4, 2021

Closes #21402
Related to influxdata/influxdb-client-csharp#190

List users via the API now supports offset, limit or after parameter

  • CHANGELOG.md updated with a link to the PR (not the Issue)
  • Well-formatted commit messages
  • Rebased/mergeable
  • Tests pass
  • http/swagger.yml updated (if modified Go structs or API)
  • Documentation updated or issue created (provide link to issue/pr)
  • Signed CLA (if not already signed)

@bednar bednar marked this pull request as ready for review May 5, 2021 11:26
@bednar bednar requested review from danxmoran and glinton May 5, 2021 11:26
Copy link
Contributor

@glinton glinton left a comment

Choose a reason for hiding this comment

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

seems reasonable

http/swagger.yml Outdated Show resolved Hide resolved
Copy link
Contributor

@danxmoran danxmoran left a comment

Choose a reason for hiding this comment

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

A suggestion on the CHANGELOG, but overall this looks great! Thanks for the contribution

CHANGELOG.md Outdated
@@ -4,6 +4,7 @@

1. [19811](https://github.com/influxdata/influxdb/pull/19811): Add Geo graph type to be able to store in Dashboard cells.
1. [21218](https://github.com/influxdata/influxdb/pull/21218): Add the properties of a static legend for line graphs and band plots.
1. [21367](https://github.com/influxdata/influxdb/pull/21367): List users via the API now supports `offset`, `limit` or `after` parameter.
Copy link
Contributor

Choose a reason for hiding this comment

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

Also mention descending here?

Or maybe something more general like List users via the API now supports pagination

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've changed it to general version: List users via the API now supports pagination.

@@ -404,11 +404,19 @@ func (h *UserHandler) handleGetUsers(w http.ResponseWriter, r *http.Request) {

type getUsersRequest struct {
filter influxdb.UserFilter
opts influxdb.FindOptions
Copy link
Contributor

Choose a reason for hiding this comment

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

I was going to suggest we not bother supporting offset, but if it's already bundled into this then it's probably easier to leave it.

@danxmoran
Copy link
Contributor

Hrm, I think the build failure is an unrelated bug in our logic to detect & avoid running certain CI steps from fork-PRs... will investigate

… `after` parameter

Signed-off-by: Jakub Bednar <jakub.bednar@gmail.com>
Signed-off-by: Jakub Bednar <jakub.bednar@gmail.com>
… `after` parameter

Signed-off-by: Jakub Bednar <jakub.bednar@gmail.com>
Signed-off-by: Jakub Bednar <jakub.bednar@gmail.com>
@bednar bednar merged commit 4becb6d into influxdata:master May 12, 2021
@bednar bednar deleted the feat/users-limit-offset-after branch May 12, 2021 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add pagination support to /users API
3 participants