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.

Sign up for GitHub

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

Jump to bottom

Aws provider apigateway schema #8383

Merged
merged 10 commits into from Oct 14, 2020
Merged

Aws provider apigateway schema #8383

merged 10 commits into from Oct 14, 2020

Conversation

thewizarodofoz
Copy link
Contributor

@thewizarodofoz thewizarodofoz commented Oct 11, 2020 •
edited by medikoo

Closes: #8018

@codecov-io
Copy link

codecov-io commented Oct 11, 2020 •
edited

Codecov Report

Merging #8383 into master will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8383      +/-   ##
==========================================
- Coverage   88.02%   88.00%   -0.02%     
==========================================
  Files         249      249              
  Lines        9278     9264      -14     
==========================================
- Hits         8167     8153      -14     
  Misses       1111     1111
Impacted Files Coverage Δ
...s/package/compile/events/apiGateway/lib/apiKeys.js 100.00% <ø> (+5.26%) ⬆️
lib/plugins/aws/lib/naming.js 98.15% <100.00%> (ø)
...s/package/compile/events/apiGateway/lib/restApi.js 83.87% <100.00%> (-14.01%) ⬇️
...age/compile/events/apiGateway/lib/usagePlanKeys.js 100.00% <100.00%> (+2.85%) ⬆️
lib/plugins/aws/provider/awsProvider.js 93.43% <100.00%> (+0.36%) ⬆️
lib/classes/ConfigSchemaHandler/index.js 89.56% <0.00%> (-0.62%) ⬇️
lib/plugins/aws/lib/monitorStack.js 100.00% <0.00%> (ø)
lib/plugins/aws/package/compile/events/s3/index.js 100.00% <0.00%> (ø)
...ins/aws/package/compile/events/apiGateway/index.js 100.00% <0.00%> (ø)
... and 5 more

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 661caad...80f189e. Read the comment docs.

medikoo
medikoo suggested changes Oct 12, 2020
View reviewed changes
Copy link
Contributor

@medikoo medikoo left a comment

Choose a reason for hiding this comment

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

Thank you @thewizarodofoz ! Looks really good, please see my remarks

lib/plugins/aws/provider/awsProvider.js Show resolved Hide resolved
lib/plugins/aws/provider/awsProvider.js Show resolved Hide resolved
lib/plugins/aws/provider/awsProvider.js Outdated
Comment on lines 35 to 36
limit: { type: 'integer' },
offset: { type: 'integer' },
Copy link
Contributor

Choose a reason for hiding this comment

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

It'll be nice to restrict minimum to 0

lib/plugins/aws/provider/awsProvider.js Outdated
additionalProperties: false,
},
},
required: [],
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's not add empty required

medikoo
medikoo reviewed Oct 14, 2020
View reviewed changes
Copy link
Contributor

@medikoo medikoo left a comment

Choose a reason for hiding this comment

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

Thank you @thewizarodofoz looks great! I've just commented on one last minor nuance, and we're ready to go!

lib/plugins/aws/provider/awsProvider.js Outdated
Comment on lines 44 to 45
burstLimit: { type: 'integer' },
rateLimit: { type: 'integer' },
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's also put minimum: 0 on those

medikoo
medikoo approved these changes Oct 14, 2020
View reviewed changes
Copy link
Contributor

@medikoo medikoo left a comment

Choose a reason for hiding this comment

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

Thank you @thewizarodofoz !

@medikoo medikoo merged commit e51e0f2 into serverless:master Oct 14, 2020
@medikoo medikoo mentioned this pull request Oct 14, 2020
Merged
@thewizarodofoz thewizarodofoz deleted the aws-provider-apigateway-schema branch October 14, 2020 12:30
@medikoo medikoo mentioned this pull request Oct 14, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@medikoo medikoo medikoo approved these changes

Assignees

@thewizarodofoz thewizarodofoz

Labels
cat/aws-event-api-gateway cat/config-schema enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Config schema: Define AWS "http" function event properties
3 participants
@thewizarodofoz @codecov-io @medikoo