diff --git a/awscli/formatter.py b/awscli/formatter.py index e13b6fd72c62..a4cd3a789069 100644 --- a/awscli/formatter.py +++ b/awscli/formatter.py @@ -231,6 +231,20 @@ def __call__(self, command_name, response, stream=None): if stream is None: stream = self._get_default_stream() try: + # For paginated responses + a JMESPath query, fully buffer the + # result before applying the query. + # + # Streaming per-page + applying the query per-page can yield + # confusing extra output when later pages do not contain the + # queried field (for example: a scalar query printing a trailing + # "None" line). Buffering keeps text output consistent with the + # JSON/table formatters. + if is_response_paginated(response) and self._args.query is not None: + response_data = response.build_full_result() + self._remove_request_id(response_data) + self._format_response(response_data, stream) + return + if is_response_paginated(response): result_keys = response.result_keys for i, page in enumerate(response): @@ -246,8 +260,6 @@ def __call__(self, command_name, response, stream=None): ) self._format_response(current, stream) if response.resume_token: - # Tell the user about the next token so they can continue - # if they want. self._format_response( {'NextToken': {'NextToken': response.resume_token}}, stream, @@ -256,8 +268,6 @@ def __call__(self, command_name, response, stream=None): self._remove_request_id(response) self._format_response(response, stream) finally: - # flush is needed to avoid the "close failed in file object - # destructor" in python2.x (see http://bugs.python.org/issue11380). self._flush_stream(stream) def _format_response(self, response, stream): diff --git a/tests/unit/test_text_formatter_pagination.py b/tests/unit/test_text_formatter_pagination.py new file mode 100644 index 000000000000..efcd6fda4223 --- /dev/null +++ b/tests/unit/test_text_formatter_pagination.py @@ -0,0 +1,44 @@ +import io + +import jmespath +from botocore.paginate import PageIterator + +from awscli.formatter import TextFormatter + + +class FakePageIterator(PageIterator): + # We intentionally don't call the parent constructor; this object is only + # used to satisfy `isinstance(response, PageIterator)` and provide a stable + # `build_full_result()` for the formatter. + def __init__(self, full_result): + self._full_result = full_result + + def build_full_result(self): + return self._full_result + + +class Args: + def __init__(self, query): + self.query = query + + +def test_text_formatter_buffers_paginated_response_when_query_is_set(): + # Regression for aws/aws-cli#10061: + # when a response is paginated, text output previously applied the query + # per-page. If later pages didn't include the queried scalar field, the + # formatter could emit an extra trailing "None" line. + args = Args(query=jmespath.compile('ChangeSetId')) + formatter = TextFormatter(args) + + response = FakePageIterator( + { + 'ResponseMetadata': {'RequestId': 'abc'}, + 'ChangeSetId': 'arn:aws:cloudformation:us-east-1:000000000000:changeSet/change-set-1/11111111111111111111111111', + 'NextToken': None, + } + ) + + stream = io.StringIO() + formatter('cloudformation describe-change-set', response, stream=stream) + + assert stream.getvalue().strip() == response._full_result['ChangeSetId']