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

Lambda auth minor fixes #8741

Merged
merged 5 commits into from Nov 9, 2021

Conversation

sundersc
Copy link
Contributor

@sundersc sundersc commented Nov 8, 2021

Description of changes

  • Use function category to create the lambda auth rather than creating it within api package.
  • Enhance function category to pass the defaultRuntime and template.
  • Add GraphQL Lambda Authorizer to the list of NodeJS templates.
  • Prompt user to edit the function created through amplify add and update api workflow.

Description of how you validated changes

  • yarn test
  • e2e test in local

Checklist

  • PR description included
  • yarn test passes
  • Tests are changed or added
  • Relevant documentation is changed or added (and PR referenced)
  • New AWS SDK calls or CloudFormation actions have been added to relevant test and service IAM policies

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

@sundersc sundersc requested a review from a team as a code owner November 8, 2021 22:30
@sundersc sundersc marked this pull request as draft November 8, 2021 22:30
@sundersc sundersc marked this pull request as ready for review November 8, 2021 22:38
context.print.success('Available advanced settings:');
advancedSettingsList.forEach(setting => context.print.info('- '.concat(setting)));
context.print.info('');
if(!templateParameters.skipAdvancedSection) {
Copy link
Member

Choose a reason for hiding this comment

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

nit: can we reverse this condition and early return to avoid more nesting?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated as suggested. Also, updated the skipNextSteps method to follow the same code style.

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.

couple nits but LGTM

print.info(
'"amplify publish" builds all of your local backend and front-end resources (if you added hosting category) and provisions them in the cloud',
);
print.success(`Successfully added resource ${completeParams.resourceName} locally.`);
Copy link
Member

Choose a reason for hiding this comment

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

nit: would be nice to switch to amplify-prompts for these print statements

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've few more fast follow tasks related to this module, I will include this.

@codecov-commenter
Copy link

Codecov Report

Merging #8741 (28f539a) into master (8c73713) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8741      +/-   ##
==========================================
- Coverage   56.88%   56.88%   -0.01%     
==========================================
  Files         756      756              
  Lines       42247    42252       +5     
  Branches     8650     8656       +6     
==========================================
  Hits        24033    24033              
- Misses      17393    17397       +4     
- Partials      821      822       +1     
Impacted Files Coverage Δ
...mation/service-walkthroughs/appSync-walkthrough.ts 14.62% <0.00%> (+0.15%) ⬆️
...tion/src/provider-utils/awscloudformation/index.ts 13.15% <0.00%> (-0.12%) ⬇️
...rmation/service-walkthroughs/lambda-walkthrough.ts 19.69% <0.00%> (-0.31%) ⬇️
...ls/awscloudformation/utils/functionPluginLoader.ts 6.36% <0.00%> (-0.31%) ⬇️
...vider-utils/awscloudformation/utils/layerParams.ts 100.00% <ø> (ø)

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 8c73713...28f539a. Read the comment docs.

@sundersc sundersc merged commit 8ff558b into aws-amplify:master Nov 9, 2021
ammarkarachi added a commit that referenced this pull request Nov 10, 2021
sachscode pushed a commit that referenced this pull request Nov 10, 2021
@github-actions github-actions bot added the referenced-in-release Issues referenced in a published release changelog label Nov 11, 2021
@github-actions
Copy link

👋 Hi, this pull request was referenced in the v6.4.0 release!

Check out the release notes here https://github.com/aws-amplify/amplify-cli/releases/tag/v6.4.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
referenced-in-release Issues referenced in a published release changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants