Bug 1142258 - hammer --csv silently truncates output when redirecting STDOUT
Summary: hammer --csv silently truncates output when redirecting STDOUT
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Satellite
Classification: Red Hat
Component: Hammer
Version: 6.0.0
Hardware: Unspecified
OS: Unspecified
medium
medium
Target Milestone: Unspecified
Assignee: Ohad Levy
QA Contact: Corey Welton
URL:
Whiteboard: Verified in Upstream
: 1179096 1179361 (view as bug list)
Depends On: 1179096
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-09-16 13:01 UTC by Nick Strugnell
Modified: 2020-01-17 15:28 UTC (History)
9 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-07-27 11:05:31 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)

Description Nick Strugnell 2014-09-16 13:01:50 UTC
Description of problem:
Hammer silently truncates output to 22 lines when redirecting standard output.

Version-Release number of selected component (if applicable):
rubygem-hammer_cli-0.1.1-5.el6sat.noarch but also affects GA version.

How reproducible:
every time

Steps to Reproduce:
1. hammer --csv template list > /tmp/templates.csv
2. wc -l /tmp/templatex.csv
3.

Actual results:
21

Expected results:
42 (the actual number of templates on my system)


Additional info:
This is due to a poor implementation of the pager In any case, the pager should always be switched off when stdout is redirected, rather than simply silently truncating the output and exiting.

Workaround:
specifying:
hammer --csv template list --per-page 9999 > /tmp/templates.csv

will list all templates (up to a maximum of 9999) - however this is not a satisfactory solution, particularly in the case of hosts where we may have more than 9999 hosts.

Comment 1 RHEL Program Management 2014-09-16 13:13:31 UTC
Since this issue was entered in Red Hat Bugzilla, the release flag has been
set to ? to ensure that it is properly evaluated for this release.

Comment 3 Dominic Cleal 2014-09-16 13:40:12 UTC
Connecting redmine issue http://projects.theforeman.org/issues/3898 from this bug

Comment 4 Bryan Kearney 2014-12-28 13:03:54 UTC
Moving to POST since upstream bug http://projects.theforeman.org/issues/3898 has been closed
-------------
Anonymous
Applied in changeset commit:hammer-cli-foreman|ae520bbb7bccbc38ba55938fb5f05c3cb5a71298.

Comment 5 Tomas Strachota 2015-01-05 14:54:32 UTC
Moving back to new since the related PR actually fixed something different. Please see the upstream issue for more details.

Comment 6 Brandon Williams 2015-01-05 15:46:24 UTC
Tomas,

Should this also address the following upstream request?

http://projects.theforeman.org/issues/3652

If a user is attempting to report on 30,000+ systems, would they just need to use the example posted by Tom:

current_results = get first page
all results += current_results
while current_results.size < per_page
current_results = get next page
all_results += current_results
end

And set the per_page option to how many systems they want to receive in each batch? Is the above functionality possible by this commit: hammer-cli-foreman|ae520bbb7bccbc38ba55938fb5f05c3cb5a71298?

Thanks,
Brandon

Comment 7 Martin Jackson 2015-01-05 17:57:29 UTC
Thanks, Brandon.  (This is Marty; I'm using bugzi credentials that predate the account.)

Tomas, we've got code that does this pagination.  It's relatively obvious how to do that.

My question is, what are the semantics in the API for that?  What http headers/cookies need to be set so that the API knows how to deal with the result set?

If, for example, we sent repeated requests via curl (I wouldn't), what I expect is that each paginated request would generate the complete result set, and only give the paginated piece we're asking for for that request.  Similarly, I don't expect that such requests would be transactional, in that if boxes were added to or removed from the result set, that the pagination approach would be racy for them:  request 1 generates 100 results; request 2 (2 seconds later) generates 150.  With the pagination approach (say 50 per request), which set am I going to get, or am I going to get the first 50, then the next 50, then the next 50 (if there are 50 left).  Granted the "window" for such problems is fairly limited, but we are evaluating just how much operational tooling we can build around the foreman API and this is an important piece of data for us.

I hope these questions make sense.  I fear I haven't asked very well.

Comment 8 Tomas Strachota 2015-01-06 08:11:19 UTC
Brandon, your proposed algorithm was possible even before the mentioned commit. The commit actually only switches off the interactive "show more?" prompt in csv mode.

The problem with your algorithm is what Marty outlined - the api queries are not transactional. The api takes parameters:
 page - page to start from
 per_page - number of items to show per page
See the docs for listing hosts in [1].

So if an item is added (or removed) between query for page 1 and page 2 there can be overlap (or items missing). The solution is to add direct support for listing items as [2] proposes. I'm going to clone [2] into BZ and reference it with this bug.

Marty, did it answer your question?

[1] http://www.theforeman.org/api/apidoc/v2/hosts/index.html
[2] http://projects.theforeman.org/issues/3652

Comment 9 Martin Jackson 2015-01-06 12:53:34 UTC
Tomas, yes, that answers my question.  Thank you!   I've added myself to the other bug so I can follow it.

Foreman is a truly excellent piece of work, by the way.  I really appreciate all the work you and the team have done on it.

Comment 10 Bryan Kearney 2015-02-18 23:05:15 UTC
Upstream bug assigned to tcaspy

Comment 11 Tom Caspy 2015-02-19 08:13:57 UTC
this is not a bug.
the data can't return without pagination (imagine a query returning 100K results, which is definitely possible)
the idea is that the machine reading the data will go through the pagination programatically.
i.e. this pseudo code:

current_results = get first page
all results += current_results
while current_results.size < per_page
current_results = get next page
all_results += current_results
end

Comment 12 Martin Jackson 2015-02-21 00:36:37 UTC
I can imagine it just fine.

Do you really expect all of your users to do this?  Shouldn't foreman be expected to be able to report on its entire node set?

Most data processing tools are easily able to handle 100k+ rows.

Comment 14 Bryan Kearney 2015-04-30 12:02:10 UTC
*** Bug 1179361 has been marked as a duplicate of this bug. ***

Comment 15 Nick Strugnell 2015-04-30 15:01:55 UTC
It's a bug - this is a command line tool and command line tools in UNIX environments should operate with standard command pipelines and redirection.

And yes, I fully expect the command to return 100,000 lines if that is how many systems there are registered.

Comment 16 Bryan Kearney 2015-04-30 18:38:45 UTC
*** Bug 1179096 has been marked as a duplicate of this bug. ***

Comment 18 Bryan Kearney 2015-04-30 18:40:45 UTC
Per all teh comments... I agree.. we need to support this. I am fine with a command line parameter having to be used ti disable pagination. That it is a valid requirement. However, the option needs to be there.

Comment 19 Tom Caspy 2015-05-04 14:38:28 UTC
okay, I'll think of a way to support this, will need to find a way to make this memory efficient...

Comment 21 Tazim Kolhar 2015-10-21 10:26:03 UTC
*** This bug is verified in upstream.  This fix should eventually land in future downstream builds ***
Version Tested:
# rpm -qa  | grep foreman
nec-em17.rhts.eng.bos.redhat.com-foreman-client-1.0-1.noarch
foreman-1.11.0-0.develop.201510121538gitb6b977a.el7.noarch
tfm-rubygem-hammer_cli_foreman_docker-0.0.3-4.el7.noarch
nec-em17.rhts.eng.bos.redhat.com-foreman-proxy-client-1.0-1.noarch
tfm-rubygem-hammer_cli_foreman-0.4.0-1.201510071112git33fd59b.el7.noarch
foreman-debug-1.11.0-0.develop.201510121538gitb6b977a.el7.noarch
foreman-release-1.11.0-0.develop.201510121538gitb6b977a.el7.noarch
foreman-postgresql-1.11.0-0.develop.201510121538gitb6b977a.el7.noarch
foreman-vmware-1.11.0-0.develop.201510121538gitb6b977a.el7.noarch
tfm-rubygem-foreman_hooks-0.3.9-1.el7.noarch
tfm-rubygem-foreman-tasks-0.7.6-1.fm1_10.el7.noarch
tfm-rubygem-hammer_cli_foreman_tasks-0.0.8-1.el7.noarch
tfm-rubygem-foreman_bootdisk-6.0.0-2.fm1_10.el7.noarch
foreman-release-scl-1-1.el7.x86_64
foreman-libvirt-1.11.0-0.develop.201510121538gitb6b977a.el7.noarch
foreman-selinux-1.11.0-0.develop.201510071426git6234447.el7.noarch
foreman-ovirt-1.11.0-0.develop.201510121538gitb6b977a.el7.noarch
tfm-rubygem-hammer_cli_foreman_bootdisk-0.1.3-3.el7.noarch
tfm-rubygem-foreman_gutterball-0.0.1-3.el7.noarch
nec-em17.rhts.eng.bos.redhat.com-foreman-proxy-1.0-2.noarch
tfm-rubygem-foreman_discovery-4.1.0-1.fm1_10.el7.noarch
tfm-rubygem-foreman_docker-1.4.1-2.fm1_10.el7.noarch
foreman-proxy-1.11.0-0.develop.201510120849git5f36f2e.el7.noarch
foreman-compute-1.11.0-0.develop.201510121538gitb6b977a.el7.noarch
foreman-gce-1.11.0-0.develop.201510121538gitb6b977a.el7.noarch

steps:
#  hammer -u admin -p changeme --csv template list > /tmp/templates.csv
# wc -l /tmp/templates.csv
55 /tmp/templates.csv

shows the actual number of templates on my system . so nothing is truncated

Comment 26 Bryan Kearney 2016-07-27 11:05:31 UTC
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.