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.

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

New serverless pattern - SQS-EBPipes-ECS-Task Typescript CDK #2217

Conversation

virajpadte
Copy link
Contributor

Description of changes:
Adding a new pattern for triggering ECS Fargate tasks from matches SQS events via EventBridge Pipes.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

eventbridge-pipes-sqs-to-ecs-task-cdk-typescript/README.md Outdated Show resolved Hide resolved
eventbridge-pipes-sqs-to-ecs-task-cdk-typescript/README.md Outdated Show resolved Hide resolved
eventbridge-pipes-sqs-to-ecs-task-cdk-typescript/README.md Outdated Show resolved Hide resolved
eventbridge-pipes-sqs-to-ecs-task-cdk-typescript/README.md Outdated Show resolved Hide resolved
eventbridge-pipes-sqs-to-ecs-task-cdk-typescript/README.md Outdated Show resolved Hide resolved
image: ContainerImage.fromRegistry('public.ecr.aws/docker/library/alpine:edge'),
logging: LogDriver.awsLogs({
streamPrefix: '/ecstask',
// logRetention: RetentionDays.ONE_DAY,
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this commented out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed


// Give pipeRole permissions to consume from the SQS queue and run the ECS task
sqsQueue.grantConsumeMessages(pipeRole);
// sqsEventBridgePipeTargetLogGroup.grantWrite(pipeRole);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this commented out, can it be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed!

// create a EventBridge pipe which consumes events from SQS with batch size 1 and delivers events to CloudWatch log group sqsEventBridgePipeTargetLogGroup
const pipe = new pipes.CfnPipe(this, 'SQSEventBridgePipe', {
logConfiguration: {
level: 'TRACE',
Copy link
Contributor

Choose a reason for hiding this comment

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

is that necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No I had this in there during debugging I will clean this up and clean it all up.

}
},
roleArn: pipeRole.roleArn,
description: 'SQS EventBridge Pipe',
Copy link
Contributor

Choose a reason for hiding this comment

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

That description is not really descriptive

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed!


// example test. To run these tests, uncomment this file along with the
// example resource in lib/eventbridge-pipes-sqs-to-ecs-task-cdk-typescript-stack.ts
test('SQS Queue Created', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you plan on adding a test here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no removed the folder

virajpadte and others added 7 commits April 8, 2024 11:18
Co-authored-by: Ben <9841563+bfreiberg@users.noreply.github.com>
Co-authored-by: Ben <9841563+bfreiberg@users.noreply.github.com>
Co-authored-by: Ben <9841563+bfreiberg@users.noreply.github.com>
Co-authored-by: Ben <9841563+bfreiberg@users.noreply.github.com>
Co-authored-by: Ben <9841563+bfreiberg@users.noreply.github.com>
… and remove unwanted trace logging configuration
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this file can be removed as you have no tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed!

@bfreiberg
Copy link
Contributor

Looks good, thanks for your contribution. A Developer Advocate will merge the changes and publish the pattern to serverlessland.com.

@julianwood julianwood merged commit 9995596 into aws-samples:main May 20, 2024
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.

None yet

4 participants