Skip to content

change default aws linux instance type to t3.micro, reduce default template TTL #2776

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

Merged
merged 5 commits into from
Jul 12, 2022

Conversation

johnstcn
Copy link
Member

@johnstcn johnstcn commented Jul 1, 2022

No description provided.

@johnstcn johnstcn self-assigned this Jul 1, 2022
@johnstcn johnstcn marked this pull request as ready for review July 1, 2022 15:56
@johnstcn johnstcn requested a review from a team July 1, 2022 15:57
- make default workspace autostop 2 hours unless specified otherwise
- add instance type selector to aws templates
@johnstcn johnstcn requested review from kylecarbs and spikecurtis July 1, 2022 21:44
Copy link
Member

@mafredri mafredri left a comment

Choose a reason for hiding this comment

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

Default TTL of 2h sounds a bit short, but I understand why we're making the change, so all good 👍🏻.

@@ -36,6 +36,22 @@ variable "region" {
}
}

variable "instance_type" {
description = "What instance type should your workspace use?"
default = "2C/1G: t3.micro"
Copy link
Member

Choose a reason for hiding this comment

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

I've had a few instances where defining the default skipped the prompt. I'm guessing this is not the case here? (Just checking.)

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't recall it being the case when I tested it!

Copy link
Member

@mafredri mafredri Jul 12, 2022

Choose a reason for hiding this comment

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

Then it's probably fine 👍, could've been related to variables marked sensitive (and that behavior might've changed already, it was a while ago).

validation {
condition = contains([
"2C/1G: t3.micro",
"2C/2G: t3.small",
Copy link
Contributor

Choose a reason for hiding this comment

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

It wasn't immediately clear what "C" and "G" meant in this context. I eventually realized it was vCPU and GB RAM, but I think 2 vCPU / 2 GB RAM is clearer and it's not like we're hurting for space in this context, nor do the users have to type these values in our UX.

Unfortunately the vCPU number is a little misleading for T3 instances. Micro-large have the same number of vCPUs, and reading this you might conclude that they have the same CPU performance. They do not. Amazon throttles the CPUs except for short bursts and the amount of throttling is different for the different instance sizes. I'm not really sure what to suggest except to drop the stats and just include the instance type names. Up to you.

Copy link
Member Author

Choose a reason for hiding this comment

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

I can drop the stats, I only put them in there so the instance types would show up in the right order :D

Copy link
Member

Choose a reason for hiding this comment

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

I guess you could do numbered list, 1. t3.micro, 2. t3.small if you want to maintain order? 😄

@johnstcn johnstcn merged commit 5e63201 into main Jul 12, 2022
@johnstcn johnstcn deleted the cj/template-ttl-default branch July 12, 2022 18:38
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.

4 participants