-
Notifications
You must be signed in to change notification settings - Fork 4.2k
fix(apigateway): correct JsonSchema.additionalItems
property type
#33879
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(apigateway): correct JsonSchema.additionalItems
property type
#33879
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(This review is outdated)
JsonSchema
additionalItems interface
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #33879 +/- ##
==========================================
+ Coverage 83.98% 84.00% +0.01%
==========================================
Files 120 121 +1
Lines 6976 6984 +8
Branches 1178 1179 +1
==========================================
+ Hits 5859 5867 +8
Misses 1005 1005
Partials 112 112
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
I think that the CodeBuild check is failing because this is a breaking API change. I'm not sure how to use |
2e19b50
to
a11b526
Compare
@pahud , good morning! Do you have time to take a look at this? I don't want the PR automation to start threatening to auto-close this PR. |
Hey, that's neat ☝️ . We've blown past the 90th percentile 😆. @pahud do you have time to have a look? It's a pretty straight forward update - just need to have a conversation with whoever is comfortable looking at the break to verify it is acceptable and correct, I hope. Please let me know any way I can help get this moving. |
Hi @jusdino I won't be able to review your PR but I will bring this up to the team for input here. Meanwhile, you can reach out to cdk.dev slack community for community inputs as well. |
By the way, as long as your PR passes the CI, it will be in the queue for community review followed by maintainer review. Check this document for more details: https://github.com/aws/aws-cdk/wiki/CDK-Community-PR-Reviews |
Yeah, that's kind of what I'm worried about - I can't make this pass CI, since it is a breaking change ☝️. I'll reach out to the slack community and see what mileage I can get. Thanks for the responses! |
Co-authored-by: Glib Shpychka <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this isn't a real review -- just a stab at what you might need to get this to pass the build. sorry we are being very obtuse about what gets in the queue for review and what doesn't.
JsonSchema
additionalItems interfaceJsonSchema
additionalItems interface
JsonSchema
additionalItems interfaceJsonSchema
additionalItems interface is undeployable
Co-authored-by: Kaizen Conroy <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @jusdino ! Thank you for making the issue and PR.
A unit test would need to be added to check that RestApi.addModel
will behave correctly when a boolean or object additionalItems
is passed. The test would just check that the additionalItems
within the CloudFormation properties is set correctly. The test suite for that is in restapi.test.ts
.
JsonSchema
additionalItems interface is undeployableJsonSchema.additionalItems
property type
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Comments on closed issues and PRs are hard for our team to see. |
Issue #33878
Closes #33878.
Reason for this change
Fix a bug in the JsonSchema interface.
🚨 This is a breaking change, but should be acceptable, since the bug prevented use of the changed interface portion 🚨
Description of changes
Changing the
JsonSchema.additionalItems
type fromJsonSchema[]
toJsonSchema | boolean
to match Json Schema Draft-04. This enables deployment of API Gateway models that include theadditionalItems
property.Describe any new or updated permissions being added
None
Description of how you validated changes
Added an integration test to validate that it fixes the deployment issue.
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license