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

Don't print response only if an empty dict/list/str #1496

Merged
merged 3 commits into from
Sep 16, 2015

Conversation

jamesls
Copy link
Member

@jamesls jamesls commented Sep 10, 2015

The integer 0 should be printed as a result. This can happen
from a JMESPath expression, e.g length(something).

For example, currently:

$ aws ec2 describe-security-groups --query 'length(`[0,1]`)'
2
$ aws ec2 describe-security-groups --query 'length(`[0]`)'
1
$ aws ec2 describe-security-groups --query 'length(`[]`)'
<prints nothing>
$

Now you'll get:

$ aws ec2 describe-security-groups --query 'length(`[]`)'
0

cc @kyleknap @mtdowling @rayluo

The integer 0 should be printed.  This can happen
from a JMESPath expression, e.g `length(something)`.
@@ -91,10 +92,11 @@ def _format_response(self, command_name, response, stream):
# the response will be an empty string. We don't want to print
# that out to the user but other "falsey" values like an empty
# dictionary should be printed.
if response:
Copy link
Member

Choose a reason for hiding this comment

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

Would it be possible to just do if response or response == 0:, or are we worried about other types that are falsey but should show up in the output?

Copy link
Member Author

Choose a reason for hiding this comment

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

After looking at this again, I think we can just change this to explicitly check for '{}'.

The original intent of this (years ago) was that any AWS operation that did not have an associated output shape should not generate any CLI output. In older version of the CLI, some operations could generate either an empty dict or empty string.

However, with the switchover to the normalized model parser, anytime an operation does not have an output shape, botocore gives back a dictionary that only has the RequestMetadata key populated. Once the formatter's here strip that key out, we're left with an empty dictionary.

So it should be safe to update this to only check for an empty dictionary. This meets the constraint that anything without an output shape should not generate CLI output.

Does that work for you?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense. By the way, will we also document this philosophy somewhere? Such as in the code as a comment paragraph?

@rayluo rayluo added the feature-request A feature should be added or improved. label Sep 14, 2015
@@ -23,6 +23,7 @@
PY3 = six.PY3
queue = six.moves.queue
shlex_quote = six.moves.shlex_quote
string_types = six.string_types
Copy link
Member

Choose a reason for hiding this comment

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

This is no longer necessary

@jamesls
Copy link
Member Author

jamesls commented Sep 16, 2015

@mtdowling Feedback incorporated.

@mtdowling
Copy link
Member

🚢

@jamesls jamesls merged commit d8820ac into aws:develop Sep 16, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants