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: adding JSON stringify to example python function #6493

Merged
merged 1 commit into from
Mar 19, 2021

Conversation

ashleyoldershaw
Copy link
Contributor

We need a JSON string for the body in order to access it in our response message. See this comment: aws-amplify/amplify-js#6390 (comment)

I had this issue with my own function which was returning null on my JSON.stringify(response). I found the above comment and it made it work! i.e. I could access the return body.

Issue #, if available:
No issue created as it's a one-line change.

Description of changes:
Adding json library import and json.dumps() call in packages/amplify-python-function-template-provider/resources/hello-world/index.py

Updating relevant unit test in packages/amplify-e2e-tests/src/__tests__/function_3.test.ts by adding a JSON.stringify() call to match the json.dumps() call in the source file.

If there's an issue with this PR let me know - this is my first open-source contribution!

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

We need a JSON string for the body in order to access it in our response message. See this comment: aws-amplify/amplify-js#6390 (comment)

I had this issue with my own function which was returning null on my JSON.stringify(response). I found the above comment and it made it work! i.e. I could access the return body.
@ashleyoldershaw ashleyoldershaw requested a review from a team as a code owner January 27, 2021 18:14
@codecov
Copy link

codecov bot commented Jan 27, 2021

Codecov Report

Merging #6493 (268679b) into master (525deb6) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #6493   +/-   ##
=======================================
  Coverage   57.28%   57.28%           
=======================================
  Files         479      479           
  Lines       21692    21692           
  Branches     4311     4311           
=======================================
  Hits        12426    12426           
  Misses       8385     8385           
  Partials      881      881           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 525deb6...268679b. Read the comment docs.

@jhockett jhockett added the pending-review Pending review from core-team label Jan 28, 2021
@ashleyoldershaw
Copy link
Contributor Author

Hi there,

Is there anything I need to do to get this approved? It is a very small change so should only take a minute to review, but could save a lot of headache for developers working with Python functions on their amplify backend :)

Copy link
Member

@edwardfoyle edwardfoyle left a comment

Choose a reason for hiding this comment

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

Hi @ashleyoldershaw thanks for the contribution! LGTM, we'll get this merged soon.

@ashleyoldershaw
Copy link
Contributor Author

Hi @ashleyoldershaw thanks for the contribution! LGTM, we'll get this merged soon.

Thanks, Edward! This is my first open-source contribution so excited to get it in - all the best!

@jhockett jhockett removed the pending-review Pending review from core-team label Mar 14, 2021
@ammarkarachi ammarkarachi merged commit a6584e2 into aws-amplify:master Mar 19, 2021
@github-actions
Copy link

This pull request has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server *-help channels for those types of questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants