-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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(ecs-patterns): add family name to queue processing service properties #4508
feat(ecs-patterns): add family name to queue processing service properties #4508
Conversation
Thanks so much for taking the time to contribute to the AWS CDK ❤️ We will shortly assign someone to review this pull request and help get it
|
cc4e3c3
to
aadc3e3
Compare
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
…mily name in queue processing ECS service constructs (aws#4507)
aadc3e3
to
36894c2
Compare
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
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.
Sorry for the delay in reviewing this PR! Thanks for taking the time to contribute!
packages/@aws-cdk/aws-ecs-patterns/lib/base/queue-processing-service-base.ts
Outdated
Show resolved
Hide resolved
packages/@aws-cdk/aws-ecs-patterns/lib/base/queue-processing-service-base.ts
Show resolved
Hide resolved
… inline with other taskdef properties
…enerated if not passed in
Pull request has been modified.
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
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.
Ship it! Thanks for taking the time to add this feature!
Thank you for contributing! Your pull request is now being automatically merged. |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Add support for specifying ECS task definition family name in queue processing ECS service constructs
Fixes #4507
Use Case
Task family names group versions of task definitions.
QueueProcessingEc2Service
andQueueProcessingFargateService
don't support providing service name when constructing the services.Proposed Solution
Add optional
taskDefinitionFamily: String
inQueueProcessingServiceBaseProps.ts
and use it when creating services inQueueProcessingEc2Service.ts
andQueueProcessingFargateService.ts
This approach does not break existing code.
Not sure about the direction in which we want this to head but another approach may be to introduce
taskImageOptions: QueueProcessingTaskImageOptions
alaNetworkLoadBalancedTaskImageOptions
andApplicationLoadBalancedTaskImageOptions
and move task and image related attributes fromQueueProcessingServiceBaseProps
toQueueProcessingTaskImageOptions
.This will affect attributes like
image
,logDriver
, 'environment', 'secrets' but will moveQueueProcessingServiceBase
closer to other implementations.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license