Skip to content
This repository was archived by the owner on Nov 19, 2025. It is now read-only.

Conversation

@tatsuya-ogawa
Copy link
Contributor

Related to #1019.
Even in the task definition,
I added determination of platform version so that efs can be used.


Enter [N/A] in the box, if an item is not applicable to your change.

Testing

  • Unit tests passed
  • Integration tests passed
  • Unit tests added for new functionality
  • Listed manual checks and their outputs in the comments (example)
  • Link to issue or PR for the integration tests:

Documentation

  • Contacted our doc writer
  • Updated our README

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@SoManyHs SoManyHs requested a review from bvtujo June 11, 2020 22:17
Copy link
Contributor

@bvtujo bvtujo left a comment

Choose a reason for hiding this comment

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

Looks good! Thank you for the contribution!

}

// TODO: revert to "LATEST" when latest refers to 1.4.0
if ecsParams != nil && len(ecsParams.TaskDefinition.EFSVolumes) != 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

we should only be setting the platform version when the launch type is FARGATE, otherwise EC2 tasks will fail.
See this PR for how we implement this check and test it in the service context.

@allisaurus
Copy link
Contributor

@tatsuya-ogawa thank you for contributing! We made a fix to the service logic for EFS support since #1019 , so if you can update the check and add tests as described, we should be good to accept this.

@bvtujo
Copy link
Contributor

bvtujo commented Jun 29, 2020

Hi @tatsuya-ogawa-- Thanks so much for the contribution. I've picked your commit up in #1076 and added test functionality. I'm going to close this for now as your change is represented in the new PR. Please let me know if you have any questions!

@bvtujo bvtujo closed this Jun 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants