fix(integ-tests): http flattenResponse #30361
Conversation
github-actions
bot
added
bug
|
@nmussy any idea when this could be merged, reviewed? Can't say I am not looking forward to it? |
|
@kornicameister I can't give an ETA for the review and release process, but I should have the PR ready some time this week |
| let resp: HttpResponseWrapper | { [key: string]: unknown }; | ||
| if (request.flattenResponse === 'true') { | ||
| // Flatten and explode JSON fields | ||
| resp = flatten(deepParseJson({ apiCallResponse: result })); | ||
| } else { | ||
| // Otherwise just return the response as-is, without exploding JSON fields | ||
| resp = { apiCallResponse: result }; | ||
| } | ||
| console.log(`Returning result ${JSON.stringify(resp)}`); |
There was a problem hiding this comment.
It might be worth considering a refactor of the flattenResponse in the common CustomResourceHandler implementation, given the SDK handler does pretty much the same thing:
| @@ -113,7 +113,7 @@ describe('HttpHandler', () => { | |||
| const response = await processEvent({ parameters: { url: 'x' } }); | |||
|
|
|||
| // THEN | |||
| expect(response.apiCallResponse.body).toEqual({ key: 'value' }); | |||
There was a problem hiding this comment.
This change should not have any impact given apiCallResponse should only be used internally, but it is caused by the type change of the HttpHandler response from HttpResponseWrapper to HttpResponseWrapper | { [key: string]: unknown }>
aws-cdk-automation
added
the
pr/needs-community-review
|
Hi This PR has been pending for community review for a while. Please consider posting it on the #contributing channel in cdk.dev. Community members will be on the lookout there as well for possible reviews. Check How to get your P2 PR community reviewed for more details. |
|
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-cdk-automation
removed
the
pr/needs-community-review
|
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). |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #30361 +/- ##
=======================================
Coverage 80.83% 80.83%
=======================================
Files 236 236
Lines 14251 14251
Branches 2490 2490
=======================================
Hits 11520 11520
Misses 2446 2446
Partials 285 285
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
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). |
|
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 |
|
Comments on closed issues and PRs are hard for our team to see. |
Issue # (if applicable)
Closes #30327
Reason for this change
There was a difference in the behavior of SDK and HTTP integration attribute extraction with the
getAttandgetAttStringmethods.awsApiCallproperly implemented and returned JSONPath-ish values by using aflattenResponseproperty. This PR adds the same functionality tohttpApiCallDescription of changes
Added an implemented
flattenResponsein theHttpHandlercustom resourceDescription of how you validated changes
Updated integ and unit tests
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license