Fix None injection for hoisted binary arguments#10369
Open
LeSingh1 wants to merge 1 commit into
Open
Conversation
InjectingArgument.add_to_params used `pass` instead of `return` in the
None guard, so when an optional hoisted argument was omitted the code
still injected the wrapped member with a None value. The sibling
OriginalArgument.add_to_params returns on None, which is the intended
behavior. This affects `aws translate import-terminology` (--data-file)
and `aws translate translate-document` (--document-content): leaving the
optional arg off produced parameters like {'TerminologyData': {'File':
None}} instead of an empty mapping.
Added unit tests covering the None case, the wrapping case, and the
merge-with-existing case.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
InjectingArgument.add_to_paramsinawscli/customizations/binaryhoist.pyusedpassinstead ofreturnin itsNoneguard. When an optional hoisted binary argument is omitted, the method falls through and still injects the wrapped member with aNonevalue.The sibling class
OriginalArgument.add_to_params(same file) correctlyreturns onNone, which confirms the intended behavior.Impact
This affects the hoisted binary blob arguments:
aws translate import-terminology(--data-file)aws translate translate-document(--document-content)When the optional argument is omitted, the serialized parameters get polluted, e.g.
{'TerminologyData': {'File': None}}instead of staying empty.Reproduction
Fix
def add_to_params(self, parameters, value): if value is None: - pass + returnTests
Added
tests/unit/customizations/test_binaryhoist.pywith three cases (None, wrapping, merge-with-existing). The None case fails before the change and passes after. Verified locally:Developed with AI assistance and verified locally as described above.