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

[ecs-patterns] taskDefinition option different between Fargate ALB and QueueProcessing services #10909

Closed
abbottdev opened this issue Oct 16, 2020 · 4 comments
Labels
@aws-cdk/aws-ecs-patterns Related to ecs-patterns library breaking-change This issue requires a breaking change to remediate. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. ecs-patterns-v2 effort/medium Medium work item – several days of effort feature/pattern Feature requests related to high level L3 pattern libraries feature-request A feature should be added or improved. p2

Comments

@abbottdev
Copy link

The Question

The signature for creating a ApplicationLoadBalancedFargateService contains a taskDefinition parameter which is expected for a FargateTaskDefinition. However, this option is missing for a QueueProcessingFargateService.

I was expecting to be able to customise the taskRole, but there could be reasons why you dont specify it for this service - alternatively it could be an API inconsistency so thought I best raise it in case it's not just me!

Environment

  • CDK CLI Version:
  • Module Version: 1.65.0
  • Node.js Version: v12.4.0
  • OS: Ubuntu
  • Language (Version): Typesript

Other information

The documentation for the relevant option is here:
https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-ecs-patterns.ApplicationLoadBalancedFargateService.html

But missing from here:
https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-ecs-patterns.QueueProcessingFargateService.html

taskDefinition? FargateTaskDefinition The task definition to use for tasks in the service. TaskDefinition or TaskImageOptions > must be specified, but not both.
@abbottdev abbottdev added guidance Question that needs advice or information. needs-triage This issue or PR still needs to be triaged. labels Oct 16, 2020
@SomayaB SomayaB added the @aws-cdk/aws-ecs-patterns Related to ecs-patterns library label Oct 16, 2020
@NGL321
Copy link
Contributor

NGL321 commented Nov 9, 2020

Hey @abbottdev,

After looking at the source, I cant see any reason that you couldn't define taskDefinition as a prop for the QueueProcessingFargateService construct (task definition is set internally and passed same as with the ApplicationLoadBalancedFargateService.

I am going to change this to a feature-request to add the additional customization ability.

@MrArnoldPalmer / @SoManyHs is this something we could implement?

😸 😷

@NGL321 NGL321 added feature-request A feature should be added or improved. effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Nov 9, 2020
@SoManyHs
Copy link
Contributor

SoManyHs commented Nov 9, 2020

Hi @abbottdev!

Thanks for opening this issue -- you're right, we had added the taskDefinition field as part of the initializer properties for the other patterns in #4213, and must have just missed it on the QueueProcessing Services. We can definitely add this -- thanks for the catch!

@SoManyHs
Copy link
Contributor

Somewhat related: #11093

@SoManyHs SoManyHs added breaking-change This issue requires a breaking change to remediate. effort/medium Medium work item – several days of effort and removed effort/small Small work item – less than a day of effort labels Nov 10, 2020
@SomayaB SomayaB added p1 p2 and removed guidance Question that needs advice or information. p1 labels Nov 12, 2020
@ericzbeard ericzbeard added the feature/pattern Feature requests related to high level L3 pattern libraries label Apr 2, 2021
@github-actions
Copy link

This issue has not received any attention in 1 year. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added closing-soon This issue will automatically close in 4 days unless further comments are made. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels May 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-ecs-patterns Related to ecs-patterns library breaking-change This issue requires a breaking change to remediate. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. ecs-patterns-v2 effort/medium Medium work item – several days of effort feature/pattern Feature requests related to high level L3 pattern libraries feature-request A feature should be added or improved. p2
Projects
None yet
Development

No branches or pull requests

7 participants