Skip to content

Conversation

TDaglis
Copy link
Contributor

@TDaglis TDaglis commented Jul 4, 2019

Issue #1014

Description of changes:
Change the validation of method_authorizer to support the combination of AWS_IAM default authorizer and NONE method authorizer.

When using AWS_IAM permission, it is only specified in the DefaultAuthorizer. Reference. This is a special case which leads to the api_authorizers being empty and results to not being able to add

Auth:
 Authorizer: 'NONE'

to a specific function, in order to override the default authorizer and make the function public.

Description of how you validated changes:

Checklist:

  • Write/update tests
  • make pr passes
  • Update documentation
  • Verify transformed template deploys and application functions as expected
  • Add/update example to examples/2016-10-31

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

@praneetap
Copy link
Contributor

Thanks for the PR! The Travis build is failing. Once those errors are fixed we will give this a deeper review.

@keetonian keetonian closed this Jul 26, 2019
@keetonian keetonian reopened this Jul 26, 2019
@keetonian
Copy link
Contributor

Ran tests locally and they all passed. Re-running travis.

@codecov-io
Copy link

codecov-io commented Jul 26, 2019

Codecov Report

Merging #1015 into develop will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #1015   +/-   ##
========================================
  Coverage    94.81%   94.81%           
========================================
  Files           69       69           
  Lines         3276     3276           
  Branches       639      639           
========================================
  Hits          3106     3106           
  Misses          89       89           
  Partials        81       81
Impacted Files Coverage Δ
samtranslator/model/eventsources/push.py 88.6% <100%> (ø) ⬆️

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 527c190...db1b887. Read the comment docs.

@jhaenchen
Copy link

I need this fix folks, just checking in!

@praneetap
Copy link
Contributor

Thank you for the contribution! Could you please add end to end tests (Input/Output) that verifies the fix? You can take a look at this pr for guidance. Please make sure you follow the pr checklist.

@TDaglis
Copy link
Contributor Author

TDaglis commented Aug 6, 2019

I attempted to add an end to end test, but it appears that it has issues in Python2.7. I don't understand what the issue is exactly. Any direction on this would be helpful. I'll get back to it.

@TDaglis
Copy link
Contributor Author

TDaglis commented Aug 7, 2019

It appears my output files had issues with Python2.7. I fixed them and now the tests pass.

@jlhood jlhood requested a review from praneetap August 13, 2019 17:18
@praneetap praneetap requested a review from keetonian August 14, 2019 03:43
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.

5 participants