fix(ecs): services essential container exceptions thrown too soon #13240
Conversation
|
|
misterjoshua
changed the title
github-actions
bot
added
the
@aws-cdk/aws-ec2
misterjoshua
changed the title
|
P.S., I'm not sure if this is a feature, fix, or chore, so I picked fix because I don't know what I'd write in the README to pass the PR linter for a feat, but it fixes a Dx issue from my perspective. |
misterjoshua
changed the title
| if (!props.taskDefinition.defaultContainer) { | ||
| throw new Error('A TaskDefinition must have at least one essential container'); | ||
| protected onValidate() { | ||
| const validationErrors = super.onValidate(); |
There was a problem hiding this comment.
I like the change, but we're changing the way we do validations in v2, and we need new code to be compatible with the new way of doing things.
New way looks like:
this.node.addValidation({
validate: () => !this.taskDefinition.defaultContainer ? ['A taskdefinition...'] : [],
});
mergify
bot
dismissed
rix0rrr’s stale review
Pull request has been modified.
|
I looked at the v2-main branch and undid the bit where I tried to bring the addXxxCapacity validation into the same addValidation as this essential container validation so that there's not a v2 merge conflict problem. |
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
SoManyHs
added
@aws-cdk/aws-ecs
This PR defers the
A TaskDefinition must have at least one essential containererror to the validate phase.
Closes #13239
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license