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

fix: Value of QueryString in the API event should only be a string #405

Merged
merged 2 commits into from
May 31, 2018

Conversation

jfuss
Copy link
Contributor

@jfuss jfuss commented May 11, 2018

Fixes the bug reported in #404

Issue #, if available: #404

Description of changes:

APIGW only supports one value for a query string key. 0.3.0 assumed this was a list. This PR aligns the Event that is sent to Lambda on a request with how APIGW handles Query Strings.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@jfuss jfuss requested a review from sanathkr May 29, 2018 19:46
Empty dict if no query params where in the request otherwise returns a dictionary of key to value

"""
query_string_dict = {}
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Most variables in this method are named query_string_<type> or some flavor of it. This is not descriptive. You don't need the prefix because you are inside a method that processes query string parameters

Copy link
Contributor

@sanathkr sanathkr left a comment

Choose a reason for hiding this comment

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

Two minor stylistic comments. But I am approving this PR anyway because it does not impact readability/functionality of the code.

# Flask returns an ImmutableMultiDict so convert to a dictionary that becomes
# a dict(str: list) then iterate over
for query_string_key, query_string_list in dict(flask_request.args).items():
query_string_value_length = len(query_string_list)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: This variable is unnecessary. You can just do if query_string_listto know if the list is empty or not

@sanathkr sanathkr added the stage/accepted Accepted and will be fixed label May 31, 2018
@sanathkr sanathkr merged commit 8f2c54a into aws:develop May 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stage/accepted Accepted and will be fixed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants