-
Notifications
You must be signed in to change notification settings - Fork 947
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
feat: eventbridge api-d cloudevents #2063
Conversation
Hi @embano1 is this PR ready for review? It is marked as a draft and I just want to make sure. |
Thx @bfreiberg ! Will do tomorrow, this was mainly for @boyney123 to take a look since he has related work here. |
TODO:
|
Hi @embano1 , did you have a chance to work on this? |
@bfreiberg sorry for delaying this one. I'd love to have @boyney123 confirm the content of the pattern since he's writing a blog post. If this code is good, I'll amend with the example JSON, so please hold for a bit more :) |
|
||
// structured mode CloudEvent | ||
const structuredCeTransformer = { | ||
specversion: "1.0", |
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.
@embano1 should we update this to 1.0.2 is that the latest one?
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.
Nope, 1.0
is the right version to use (see comments)
nice pattern @embano1 thanks, will include link on my blog post, guess this can't go out without custom header support? |
Yes, this needs /hold for now.
|
e10802a
to
ce26c72
Compare
@boyney123 @bfreiberg finalized the code. Please kindly review. |
eventbridge-api-destinations-cloudevents-cdk-typescript/README.md
Outdated
Show resolved
Hide resolved
eventbridge-api-destinations-cloudevents-cdk-typescript/example-pattern.json
Show resolved
Hide resolved
...udevents-cdk-typescript/lib/eventbridge-api-destinations-cloudevents-cdk-typescript-stack.ts
Outdated
Show resolved
Hide resolved
|
||
// example test. To run these tests, uncomment this file along with the | ||
// example resource in lib/eventbridge-api-destinations-cloudevents-cdk-typescript-stack.ts | ||
test('SQS Queue Created', () => { |
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.
Was this supposed to be uncommented?
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.
bootstrap files :) will remove
"repoURL": "https://github.com/aws-samples/serverless-patterns/tree/main/eventbridge-api-destinations-cloudevents-cdk-typescript", | ||
"templateURL": "serverless-patterns/eventbridge-api-destinations-cloudevents-cdk-typescript", | ||
"projectFolder": "eventbridge-api-destinations-cloudevents-cdk-typescript", | ||
"templateFile": "lib/eventbridge-api-destinations-cloudevents-cdk-typescript-stack.ts" |
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.
There is an issue when the templateFile contains the projectFolder name. Could you please rename the stack file to something that doesn't contain the folder name.
Closes: aws-samples#2057 Signed-off-by: Michael Gasch <[email protected]>
@bfreiberg done |
Great, thank you for your contribution. |
Thx a ton @bfreiberg, great review! cc/ @boyney123 read to merge since this was released https://aws.amazon.com/about-aws/whats-new/2024/02/amazon-eventbridge-api-destinations-content-type-header-customization/ |
Closes: #2057
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.