Bug 1247781 - hammer queries defaults to 1000 items per page
hammer queries defaults to 1000 items per page
Status: CLOSED ERRATA
Product: Red Hat Satellite 6
Classification: Red Hat
Component: Hammer (Show other bugs)
6.1.0
Unspecified Unspecified
unspecified Severity high (vote)
: GA
: --
Assigned To: Martin Bacovsky
sthirugn@redhat.com
http://projects.theforeman.org/issues...
: Regression, Triaged
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2015-07-28 16:54 EDT by sthirugn@redhat.com
Modified: 2016-07-27 07:37 EDT (History)
11 users (show)

See Also:
Fixed In Version: rubygem-hammer_cli-0.5.1.6-1,rubygem-hammer_cli_foreman-0.5.1.8-1
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2016-07-27 07:37:02 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
hammer pagination (129.40 KB, image/png)
2016-05-20 03:42 EDT, Ondřej Pražák
no flags Details


External Trackers
Tracker ID Priority Status Summary Last Updated
Foreman Issue Tracker 14530 None None None 2016-04-22 12:28 EDT

  None (edit)
Description sthirugn@redhat.com 2015-07-28 16:54:05 EDT
Description of problem:
hammer queries default to 1000 items per page which makes hammer very slow 

Version-Release number of selected component (if applicable):
Sat 6.1 GA Snap 14

How reproducible:
Always

Steps to Reproduce:
# cat /etc/hammer/cli_config.yml 
:ui:
  :interactive: true
  :per_page: 20
  :history_file: '~/.foreman/history'

# enable/disable color output of logger in Clamp commands
:watch_plain: false

:log_dir: '~/.foreman/log'
:log_level: 'error'
:log_api_calls: false
#:log_owner: 'foreman'
#:log_group: 'foreman'
#:log_size: 5 #MB



# hammer -u admin -p changeme repository list --organization-id=3
---|-------------------------------------------------------|---------------------------------|--------------|--------------------------------------------------------------------
ID | NAME                                                  | PRODUCT                         | CONTENT TYPE | URL                                                                
---|-------------------------------------------------------|---------------------------------|--------------|--------------------------------------------------------------------
27 | Red Hat Enterprise Linux 5 Server RPMs x86_64 5Server | Red Hat Enterprise Linux Server | yum          | https://cdn.redhat.com/content/dist/rhel/server/5/5Server/x86_64/os
1  | Red Hat Enterprise Linux 7 Server RPMs x86_64 7Server | Red Hat Enterprise Linux Server | yum          | https://cdn.redhat.com/content/dist/rhel/server/7/7Server/x86_64/os
---|-------------------------------------------------------|---------------------------------|--------------|--------------------------------------------------------------------

# time hammer -u admin -p changeme erratum list
....
....
-------------------------------------|----------------|-------------|---------------------------------------------------------------------------------

real	7m58.835s
user	7m32.086s
sys	0m0.266s


Actual results:
See above that the erratum list command took almost 8 minutes to finish

Expected results:
Pagination should work

Additional info:
Comment 1 sthirugn@redhat.com 2015-07-28 17:19:54 EDT
Just to clarify, the command '# time hammer -u admin -p changeme erratum list' listed 1000 items but I was expecting to see first 20 items with options to search next.
Comment 3 Adam Price 2015-07-31 12:45:12 EDT
this still respects the page and per_page options if passed in at the command line via --page and --per_page or when included in api requests.
Comment 4 Mike McCune 2015-07-31 12:48:09 EDT
We actually changed this on purpose for 6.1 to turn of pagination by default:

https://bugzilla.redhat.com/show_bug.cgi?id=1190857

the side effect is that some API calls can return a *lot* of data.

For 6.2 we can investigate a few options:

* Timing functions where hammer will indicate the call is taking a while and possibly returning lots of data. give the user a suggestion to CTRL+C and use the --per-page option

* Have APIs offer a default pagination with the requirement to use a param to indicate you want *all* results
Comment 9 Shimon Shtein 2016-03-16 10:11:23 EDT
After https://github.com/theforeman/hammer-cli/pull/174 you can add a default option for pagination. You can even scope it to a specific resourse. I think it will cover both cases - if a user wants, he can add a pagination option, and the global default is full results.

Is it a valid workaround?
Comment 10 Brad Buckingham 2016-03-16 10:12:48 EDT
Adding needinfo for Suresh based on comment #9
Comment 11 sthirugn@redhat.com 2016-03-16 10:25:30 EDT
ACK for Comment 9 behavior.
Comment 13 Shimon Shtein 2016-03-16 12:48:44 EDT
It should be. The related BZ is https://bugzilla.redhat.com/show_bug.cgi?id=1154383 which is in ON_QA state and included in SNAP1.
Comment 14 Brad Buckingham 2016-03-16 13:58:06 EDT
Shimon, I chatted with Suresh in irc.  Since the behavior is addressed in the other BZ, we are going to move this one to ON_QA and he can run through verification.
Comment 15 sthirugn@redhat.com 2016-03-21 13:15:40 EDT
This bug failed in Satellite 6.2.0-beta-snap4 due to the same reason mentioned in https://bugzilla.redhat.com/show_bug.cgi?id=1154383#c10
Comment 16 Shimon Shtein 2016-03-22 05:23:02 EDT
Created an upstream PR to fix the issue, temporary workaround in https://bugzilla.redhat.com/show_bug.cgi?id=1154383
Comment 18 Mike McCune 2016-04-01 00:28:35 EDT
this is solved via 1154383 and associated bugs, moving ON_QA
Comment 19 sthirugn@redhat.com 2016-04-01 15:28:37 EDT
Failed in Satellite 6.2-beta-snap-6.  I don't see any per-page options supported.  Only organization_id, location_id are shown.


[root@sat6host ~] # hammer -u admin -p changeme defaults providers
---------|------------------------------|-------------------------------------------------------------
PROVIDER | SUPPORTED DEFAULTS           | DESCRIPTION                                                 
---------|------------------------------|-------------------------------------------------------------
foreman  | organization_id, location_id | Use the default organization and/or location from the server
---------|------------------------------|-------------------------------------------------------------

More test steps are also documented in https://bugzilla.redhat.com/show_bug.cgi?id=1154383#c19
Comment 21 Martin Bacovsky 2016-04-06 10:18:26 EDT
Let sum up the status:

- there is a bug in hammer causing per_page from the config file is being ignored. That is IMO the primary reason for this BZ. PR is almost ready
- I can confirm the workaround Shimon suggested works:
 $ hanner defaults add --param-name per_page --param-value 3
 $ hammer template list
---|----------------------------|----------
ID | NAME                       | TYPE     
---|----------------------------|----------
7  | Alterator default          | provision
8  | Alterator default finish   | finish   
9  | Alterator default PXELinux | PXELinux 
---|----------------------------|----------

To improve UX I'm adding message with page/total to the incomplete listings
Comment 22 Martin Bacovsky 2016-04-07 10:13:11 EDT
Created redmine issue http://projects.theforeman.org/issues/14530 from this bug
Comment 23 Bryan Kearney 2016-04-28 08:08:51 EDT
Moving to POST since upstream bug http://projects.theforeman.org/issues/14530 has been closed
Comment 24 Ondřej Pražák 2016-05-20 03:42 EDT
Created attachment 1159824 [details]
hammer pagination

I do not think this got cherrypicked right since the relevant code is missing (RHEL6, snap 12). Line 331 in hammer_cli_foreman/commands.rb should be:

https://github.com/theforeman/hammer-cli-foreman/blob/master/lib/hammer_cli_foreman/commands.rb#L331

but is:
pagination_supported? && option_per_page.nil? && option_page.nil?

the screenshot provides more detailed info
Comment 26 Tomas Strachota 2016-05-27 04:02:18 EDT
Verified with:
tfm-rubygem-hammer_cli-0.5.1.7-1.el7sat.noarch
tfm-rubygem-hammer_cli_foreman-0.5.1.8-1.el7sat.noarch

Hammer reflected on the per-page setting from config files.
Comment 27 Bryan Kearney 2016-07-27 07:37:02 EDT
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2016:1501

Note You need to log in before you can comment on or make changes to this bug.