boto3 icon indicating copy to clipboard operation
boto3 copied to clipboard

Provide meaningful string representation for DynamoDB Attr/Condition

Open rajveerappan opened this issue 2 years ago • 9 comments

Describe the feature

Currently printing/logging a FilterExpression will show something like:

<boto3.dynamodb.conditions.AttributeExists object at 0x1041021f0>
<boto3.dynamodb.conditions.And object at 0x104102370>
<boto3.dynamodb.conditions.In object at 0x1041027f0>

which is not very helpful. Implementing __str__ on the boto3 Attr and ConditionBase classes will instead result in logging like:

attribute_exists(mykey)
(mykey = foo AND myotherkey = bar)
mykey IN foo

Use Case

Improved human readable logging of DynamoDB FilterExpressions

Proposed Solution

https://github.com/boto/boto3/pull/3254

The way this is implemented is by turning an expression_format which looks like {0} {operator} {1} into something that looks like {values[0]} {operator} {values[1]} which can be passed to the string format method using named indices.

Other Information

An alternative solution would be to change all the expression_formats themselves to use a format like {values[0]} {operator} {values[1]} but that would be a larger change and require updating the end consumers of ConditionBase#get_expression.

Avoiding the regex could also be accomplished by explicitly setting a new str_expression_format field that uses a format like {values[0]} {operator} {values[1]}.

Acknowledgements

  • [X] I may be able to implement this feature request
  • [ ] This feature might incur a breaking change

SDK version used

latest

Environment details (OS name and version, etc.)

all

rajveerappan avatar May 06 '22 00:05 rajveerappan

Thanks @rajveerappan for the feature request. I’ll plan to bring this up for discussion with the team next week to get their thoughts and will share any updates here.

tim-finnigan avatar May 06 '22 21:05 tim-finnigan

Thanks for your patience - this fell off our radar but still needs to be prioritized for team review. It looks like this may actually be a duplicate of an older issue: https://github.com/boto/boto3/issues/1202. We try to consolidate overlapping issues in order to making tracking easier. Please let us know if there are any distinctions you want to highlight between these issues, otherwise I think we should continue tracking this in https://github.com/boto/boto3/issues/1202.

tim-finnigan avatar Nov 18 '22 20:11 tim-finnigan

Greetings! It looks like this issue hasn’t been active in longer than five days. We encourage you to check if this is still an issue in the latest release. In the absence of more information, we will be closing this issue soon. If you find that this is still a problem, please feel free to provide a comment or upvote with a reaction on the initial post to prevent automatic closure. If the issue is already closed, please feel free to open a new one.

github-actions[bot] avatar Nov 23 '22 21:11 github-actions[bot]

apologies, this fell off my radar as well. I had a PR open for this that had some feedback. I can try to clean that up.

rajveerappan avatar Nov 23 '22 21:11 rajveerappan

As I mentioned in #3608, this would also be a relatively large improvements to testing using Stubber. Today a test that fails because of mismatched conditions will have an error like (simplified):

Expected:

'KeyCondition': <boto3.dynamodb.conditions.And object at 0x104102370>

Got:

'KeyCondition': <boto3.dynamodb.conditions.And object at 0x1041027f1>

because lack of printing of e.g. the And condition. This makes it the message not much more informative than "the test failed due to some error in the condition".

Once pretty-printing for conditions is fixed we could (in some future PR) also support diffing for test failures using Stubber, like e.g. pytest does in general. Diffing the expected and actual arguments to a boto3 service call now would be not as helpful due to the pretty-printing of conditions.

tibbe avatar Mar 07 '23 05:03 tibbe

@tibbe / @tim-finnigan I finished up the PR and tests, please take a look: https://github.com/boto/boto3/pull/3254

rajveerappan avatar Mar 07 '23 16:03 rajveerappan

@rajveerappan thanks for putting it together. Added a comment that I think is relevant at https://github.com/boto/boto3/pull/3254#discussion_r1129482760

tibbe avatar Mar 08 '23 14:03 tibbe

@tim-finnigan how can we get this PR merged?

rajveerappan avatar Apr 29 '23 20:04 rajveerappan