feat(batch): fargate support for jobs#15848
feat(batch): fargate support for jobs#15848mergify[bot] merged 6 commits intoaws:masterfrom DDynamic:feature/batch-fargate-support
Conversation
|
|
| const isFargate = ComputeResourceType.FARGATE === props.computeResources.type | ||
| || ComputeResourceType.FARGATE_SPOT === props.computeResources.type; | ||
|
|
||
| if (!isFargate) { |
There was a problem hiding this comment.
Better to avoid code that bypasses the type-system if we can. You can just compute the values before initializing the computeEnvironment and use them
| }; | ||
|
|
||
| if (!isFargate) { | ||
| Object.assign(containerProperties, { |
There was a problem hiding this comment.
Same - just compute before needing to use it, and declare the types it uses so its easy to navigate and deep dive.
| /** | ||
| * Whether or not to assign a public IP to the job | ||
| */ | ||
| export enum AssignPublicIp { |
There was a problem hiding this comment.
This seems funky, can we not just convert all of this to a boolean?
|
@DDynamic Thanks for reviving this! |
|
@iliapolo should be ready for a final pass. |
|
@DDynamic found a couple of more stuff that caught my eye, but I feel good we can merge this soon enough. Thanks! |
|
@iliapolo ready for another pass. Thanks for the through review! |
|
Any chance this is going to be merged in soon? hanging out for this feature and it looks complete. |
…plify network config
|
@madeline-k 👋 I see you're now the assignee for aws-batch PRs and issues. Would you mind helping to bring this to completition? |
|
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). |
Added Fargate support for Batch jobs.
Note: this is not entirely my work - most of it was done by @kokachev. It is an updated version of Fargate support for batch jobs based on the feedback left in #13591.
ResourceRequirementsfor Fargate jobscloses: #13590, #13591
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license