Skip to content

feat(api-gateway,event-bridge): apigateway to event bus put events integration#363

Merged
thantos merged 35 commits intomainfrom
api_g_eb
Jul 28, 2022
Merged

feat(api-gateway,event-bridge): apigateway to event bus put events integration#363
thantos merged 35 commits intomainfrom
api_g_eb

Conversation

@thantos
Copy link
Copy Markdown
Collaborator

@thantos thantos commented Jul 26, 2022

Depends on #347

  • Support sending API gatgeway requests to Event Bridge
    • Caveat: only supports object literals
  • Bug: API Gateway integration strings in SFN were incorrectly stringified (from fix: apig vtl stringify #361).

@thantos thantos requested a review from sam-goodwin July 26, 2022 21:56
@netlify
Copy link
Copy Markdown

netlify bot commented Jul 26, 2022

Deploy Preview for effortless-malabi-1c3e77 ready!

Name Link
🔨 Latest commit ba087e6
🔍 Latest deploy log https://app.netlify.com/sites/effortless-malabi-1c3e77/deploys/62e29b0abd057e00085107d9
😎 Deploy Preview https://deploy-preview-363--effortless-malabi-1c3e77.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

github-actions and others added 2 commits July 26, 2022 22:12
Signed-off-by: github-actions <github-actions@github.com>
const objectProps = arg.properties.map((prop) => {
if (
!isPropAssignExpr(prop) ||
!(isIdentifier(prop.name) || isStringLiteralExpr(prop.name))
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

nit: are the parens necessary?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Would it be easier to check for what's allowed instead of the !? I am confused.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

what about numeric properties?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

image

Looks like that is our miss

export type PropertyName = Identifier | StringLiteral | NumericLiteral | ComputedPropertyName | PrivateIdentifier

test/api.test.ts Outdated
return "success";
}
)
).toThrow("object literal");
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Is this the whole message it throws?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

No, toThrow does a contains.

@thantos thantos changed the base branch from main to samg-one-to-one-ast July 27, 2022 16:59
@thantos
Copy link
Copy Markdown
Collaborator Author

thantos commented Jul 27, 2022

Rebased onto #347 to reflect the change before it gets pushed.

Base automatically changed from samg-one-to-one-ast to main July 27, 2022 18:17
Copy link
Copy Markdown
Owner

@sam-goodwin sam-goodwin left a comment

Choose a reason for hiding this comment

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

Looks like there are changes brought across from my ast change? Merge problems?

@thantos thantos merged commit 91c07ba into main Jul 28, 2022
@thantos thantos deleted the api_g_eb branch July 28, 2022 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants