Skip to content
This repository has been archived by the owner on Feb 16, 2023. It is now read-only.

Paginate audit command #215

Merged
merged 22 commits into from
Nov 27, 2019
Merged

Paginate audit command #215

merged 22 commits into from
Nov 27, 2019

Conversation

SimonBarendse
Copy link
Member

Show 10 events at a time and continue as long as the user wants
(as opposed to previously only showing 50 events).

Show 10 events at a time and continue as long as the user wants
(as opposed to previously only showing 50 events).
Creating the client will prompt the user for their passphrase when
needed. We prefer to do any validation beforehand, so that users
don't have to type their passphrase unnecessarily.
internals/secrethub/audit.go Outdated Show resolved Hide resolved
internals/secrethub/audit.go Outdated Show resolved Hide resolved
@jpcoenen
Copy link
Member

jpcoenen commented Nov 25, 2019

While testing I come across the following usability things:

  1. 10 events at a time are very few. I'd say at least 20 and more towards 50 could also be an option.
  2. I understand that from a technical perspective, redrawing the full table every time is easier but is that also the case from a usability point of view? At least the current situation looks rather cluttered. Though maybe this can be solved with point 1.
  3. What if I don't want to scroll but just want output?

@SimonBarendse
Copy link
Member Author

I've changed the default to 20 events per page and made it configurable with --per-page. Also, I've added --no-prompt to disable pagination.

internals/secrethub/audit.go Outdated Show resolved Hide resolved
internals/secrethub/audit.go Outdated Show resolved Hide resolved
internals/secrethub/audit.go Outdated Show resolved Hide resolved
internals/secrethub/audit.go Outdated Show resolved Hide resolved
internals/secrethub/audit.go Outdated Show resolved Hide resolved
This makes it clearer what is omitted
This uses tty when input is piped
@jpcoenen
Copy link
Member

jpcoenen commented Nov 25, 2019

Also, I've added --no-prompt to disable pagination.

Have you tested this? For me, it just hangs the command without any output.

@SimonBarendse
Copy link
Member Author

Also, I've added --no-prompt to disable pagination.

Have you tested this? For me, it just hangs the command without any output.

Yes, this works on my machine + repo.
When not using pagination, all events for the given repo/secret are fetched. So that might take a while.

Note that because the width of columns is determined on the size the data has for each entry in the column, output is only written after the last event has been fetched (on tabWriter.Flush).

Copy link
Member

@jpcoenen jpcoenen left a comment

Choose a reason for hiding this comment

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

One extra question: what happens in case the output of the command is piped?

internals/secrethub/audit.go Outdated Show resolved Hide resolved
internals/secrethub/audit.go Outdated Show resolved Hide resolved
internals/secrethub/audit.go Show resolved Hide resolved
@SimonBarendse
Copy link
Member Author

One extra question: what happens in case the output of the command is piped?

If tty is available, Press <ENTER> to show more results. Press <CTRL+C> to exit. is written to tty and a new line is read from tty to continue writing another page to stdout.
If tty is not available, only the first page is passed to stdout.

@SimonBarendse SimonBarendse merged commit 6260bd5 into develop Nov 27, 2019
@SimonBarendse SimonBarendse deleted the feature/audit-pagination branch November 27, 2019 13:03
@SimonBarendse SimonBarendse mentioned this pull request Nov 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants