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

Add variables for task and container CPU and memory #65

Merged
merged 5 commits into from
Jan 12, 2022

Conversation

crbudzeak
Copy link
Contributor

Ishmael asked that the CPU and memory parameters for the task and container be overridable variables. His team's use case requires a lot more of both to execute successfully.

Copy link
Contributor

@leslie-corbalt leslie-corbalt left a comment

Choose a reason for hiding this comment

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

Not related to this change, but I have a question about line 233 in ecs.tf: is the for expression doing anything?

I made 2 small comments.

variables.tf Outdated Show resolved Hide resolved
variables.tf Show resolved Hide resolved
@crbudzeak
Copy link
Contributor Author

About the for in ecs.tf. I expect that it does set the subnet IDs from the variable list. I haven't verified that.

}

variable "container_memory" {
description = "The container memroy size"
Copy link
Contributor

Choose a reason for hiding this comment

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

just saw this typo!

Copy link
Contributor

@leslie-corbalt leslie-corbalt left a comment

Choose a reason for hiding this comment

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

typo shows up in 1 more place

Copy link
Contributor

@leslie-corbalt leslie-corbalt left a comment

Choose a reason for hiding this comment

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

You fixed all typos. LGTM!

@crbudzeak crbudzeak merged commit 1a0200b into main Jan 12, 2022
@crbudzeak crbudzeak deleted the add-cpu-mem-vars branch January 12, 2022 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants