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

queryStringParameters set to None when there are no GET parameters #5893

Merged
merged 5 commits into from Apr 21, 2022

Conversation

jops-wtakase
Copy link
Contributor

On Lambda proxy integration enabled environment, if there are no GET parameters in a request path, queryStringParameters should be None instead of {}.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome to LocalStack! Thanks for raising your first Pull Request and landing in your contributions. Our team will reach out with any reviews or feedbacks that we have shortly. We recommend joining our Slack Community and share your PR on the #community channel to share your contributions with us. Please make sure you are following our contributing guidelines and our Code of Conduct.

Copy link
Member

@whummer whummer left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for this PR @jops-wtakase ! 👍 Also requesting a review from @calvernaz

Looks like the CI builds were not triggered, which can sometimes happen if the CircleCI<>Github connection is broken for your user account. Can you please try this:

  1. go to https://circleci.com,
  2. sign in with your Github credentials (Single-Sign On)
  3. come back here, push an empty commit onto the branch (should hopefully trigger the CI builds..)

Thanks!

@whummer whummer requested a review from calvernaz April 20, 2022 07:55
Copy link
Contributor

@calvernaz calvernaz left a comment

Choose a reason for hiding this comment

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

Hi @jops-wtakase thanks for pushing for this change. I agree None would be a better representation of there are no query parameters, however I think we need to change this change to this code path:

event = construct_invocation_event(

query_string_params = query_string_params or parse_request_data(method, path, "")

parse_qs returns an empty dict if there are no string parameters and that is passed into the Lambda event payload.

Copy link
Contributor

@calvernaz calvernaz left a comment

Choose a reason for hiding this comment

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

Thanks for doing this!

@whummer whummer merged commit 4553d47 into localstack:master Apr 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants