-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix: sam local start api reques to correclty support binary files in request #8018
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
base: develop
Are you sure you want to change the base?
Conversation
|
Want for #8019 to merge. This contains some fix for failing makr pr test. |
|
Waiting for #8022 to merge before rerun the workflow. |
|
I'm curious how much manual testing you did here, because the unit tests seem nice, but I don't really know if there are cases where the API request actually looks different than the mocked requests data that you added. Did you try this with Ideally you can run the But ideally we should have had a test before that should have failed because this behavior wasn't working. We do have some integ tests that check with a binary file, so it's weird that no test was catching this problem.
|
|
overall looks good, but can we keep the original test while adding new ones? |
Which issue(s) does this change fix?
#7927
Why is this change necessary?
From original issue:
How does it address the issue?
Update logic in event_constructor.py to decode request for various cases. .
What side effects does this change have?
Mandatory Checklist
PRs will only be reviewed after checklist is complete
make prpassesmake update-reproducible-reqsif dependencies were changedBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.