feat(ecs): add port mappings to containers with props #13262
Conversation
|
|
|
+1 This one looks good to me. I'll get a second opinion from @SoManyHs or @piradeepk just to be on the safe side though |
|
Looks good to me too. @misterjoshua do you mind updating one of the existing integ tests to define container port mappings in the constructor, rather than after construction? |
packages/@aws-cdk/aws-ecs/README.md
Outdated
| ```ts | ||
| taskDefinition.addContainer("WebContainer", { | ||
| image: ecs.ContainerImage.fromRegistry("amazon/amazon-ecs-sample"), | ||
| memoryLimitMiB: 1024, | ||
| portMappings: [{ containerPort: 3000 }] | ||
| }) | ||
| ``` | ||
|
|
||
| ```ts | ||
| container.addPortMappings({ | ||
| containerPort: 3000 |
There was a problem hiding this comment.
Can you add a comment above both to explain that there are 2 ways of defining the port mapping?
There was a problem hiding this comment.
Oh sure. Just saw this. Let me get right on that.
@piradeepk Done! :) |
|
@misterjoshua thanks for the patience. I was wondering if you could add one more unit test case in which you add a port mapping to the constructor and also call addPortMapping with a different port mapping (to verify that both mappings are included, and that one does not overwrite the other). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
@piradeepk This is done. Please let me know if you need any more changes. |
There was a problem hiding this comment.
This is awesome! Thanks for contributing and for addressing all of the feedback @misterjoshua!!! Ship it! 💯
|
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). |
This PR adds an optional
portMappingsproperty toContainerDefinitionOptionsso that users can add port mappings when they add a container to aTaskDefinition.Closes #13261
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license