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

Pagination in auth is broken #1748

Closed
ozkatz opened this issue Apr 11, 2021 · 1 comment · Fixed by #1755
Closed

Pagination in auth is broken #1748

ozkatz opened this issue Apr 11, 2021 · 1 comment · Fixed by #1755
Assignees
Labels
next Target for next release release-blocker

Comments

@ozkatz
Copy link
Collaborator

ozkatz commented Apr 11, 2021

auth.ListPaged expectes the struct field name to match the name of the column in the database. In our case, this is never true. This means pagination in auth is essentially broken for all paginated responses.

Reproduction is easy: calling listUsers with an amount < number of users will result in has_more: false with "next_offset": "<invalid Value>".

@ozkatz ozkatz added release-blocker next Target for next release labels Apr 11, 2021
@arielshaqed
Copy link
Contributor

Ouch. On it.

arielshaqed added a commit that referenced this issue Apr 12, 2021
Tested by fixing the test (to look at >1 word), and also by paging with amount 2 and examining
the `next_offset` field.

Fixes #1748.
johnnyaug pushed a commit that referenced this issue Apr 13, 2021
* [bug] Use DB tag to look up "next" field in auth pagination

Tested by fixing the test (to look at >1 word), and also by paging with amount 2 and examining
the `next_offset` field.

Fixes #1748.

* [bug] Return HasMore field from auth listings

Previously was always `false`, so paging was never flagged.

Tested by listing:
```
go run ./cmd/lakectl -c ~/.lakectl.nessie.yaml auth users list  --amount 2 --after five
+---------+-------------------------------+
| USER ID | CREATION DATE                 |
+---------+-------------------------------+
| four    | 2021-04-12 13:29:20 +0300 IDT |
| nessie  | 2021-04-11 17:07:59 +0300 IDT |
+---------+-------------------------------+
for more results run with --amount 2 --after "nessie"
```

The line "for more results..." never used to appear!

* [bug] Cap maximal number amount in auth listings

* Use unadorned column names

auth paginator cannot handle table-decorated column names, it looks for the a field with an
exact tag name.  Avoid unnecessary adornment of the table from which a column comes.  It is
(by definition!) not needed to describe the column, which is how the unadorned `db:...` tag
works.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
next Target for next release release-blocker
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants