Skip to content

chore(examples): remove deprecated startup_script_timeout#12104

Merged
johnstcn merged 3 commits into
mainfrom
cj/rm-startup-script-timeout
Feb 12, 2024
Merged

chore(examples): remove deprecated startup_script_timeout#12104
johnstcn merged 3 commits into
mainfrom
cj/rm-startup-script-timeout

Conversation

@johnstcn
Copy link
Copy Markdown
Member

@johnstcn johnstcn commented Feb 12, 2024 •
edited
Loading

Creating a template from examples results in deprecation warnings, which is a bit of a product smell from a user's perspective.

@johnstcn johnstcn self-assigned this Feb 12, 2024
johnstcn
johnstcn commented Feb 12, 2024
View reviewed changes
Comment thread scaletest/templates/scaletest-runner/main.tf
@johnstcn johnstcn requested review from matifali and mtojek February 12, 2024 13:46
matifali
matifali reviewed Feb 12, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@matifali matifali left a comment

Choose a reason for hiding this comment

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

What is the alternative of this deprecated timeout flags?

Comment thread scaletest/templates/scaletest-runner/main.tf Outdated
@johnstcn @matifali
Update scaletest/templates/scaletest-runner/main.tf
054265b 
Co-authored-by: Muhammad Atif Ali <atif@coder.com>
@johnstcn
Copy link
Copy Markdown
Member Author

johnstcn commented Feb 12, 2024 •
edited
Loading

What is the alternative of this deprecated timeout flags?

@mafredri feel free to correct me if I'm wrong, but I believe startup_script_behavior is the replacement: https://registry.terraform.io/providers/coder/coder/latest/docs/resources/agent#startup_script_behavior

edit: see below

@mafredri
Copy link
Copy Markdown
Member

mafredri commented Feb 12, 2024

What is the alternative of this deprecated timeout flags?

@mafredri feel free to correct me if I'm wrong, but I believe startup_script_behavior is the replacement: registry.terraform.io/providers/coder/coder/latest/docs/resources/agent#startup_script_behavior

There is no replacement, actually. And startup_script_behavior is kind of deprecated too (as per updated provider docs), both startup_script and startup_script_behavior are simply aliases for defining a coder_script with certain properties.

@matifali
Copy link
Copy Markdown
Member

matifali commented Feb 12, 2024 •
edited
Loading

It is a bit out of scope, but thoughts on replacing all arch and os attributes for the coder_agent resource with data.coder_workspace.me.arch and data.coder_workspace.me.os. Currently, I see a mix of both. We can do it in a separate PR, but we want your thoughts.

One downside of using it could be a failed workspace when workspace compute and provisionerd have different os or arch.

@johnstcn
Copy link
Copy Markdown
Member Author

johnstcn commented Feb 12, 2024

A bit out of scope but thoughts on replacing all arch and os attributes for the coder_agent resource with darta.coder_workspace.me.arch and data.coder_workspace.me.os. Currently, I see a mix of both. We can do it in a separate PR, but we would like to hear your thoughts.

One downside of using it could be when workspace compute and provisionerd have different os or arch.

Definitely something for a separate PR, but I think the case you mention is solvable with a comment above the setting.

mtojek
mtojek approved these changes Feb 12, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@mtojek mtojek left a comment

Choose a reason for hiding this comment

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

👍

@matifali
Copy link
Copy Markdown
Member

matifali commented Feb 12, 2024

What is the alternative of this deprecated timeout flags?

@mafredri feel free to correct me if I'm wrong, but I believe startup_script_behavior is the replacement: registry.terraform.io/providers/coder/coder/latest/docs/resources/agent#startup_script_behavior

There is no replacement, actually. And startup_script_behavior is kind of deprecated too (as per updated provider docs), both startup_script and startup_script_behavior are simply aliases for defining a coder_script with certain properties.

We should then also remove startup_script_behaviour.

matifali
matifali reviewed Feb 12, 2024
View reviewed changes
Comment thread scaletest/templates/scaletest-runner/main.tf
matifali
matifali reviewed Feb 12, 2024
View reviewed changes
Comment thread scaletest/terraform/k8s/coder.tf Outdated
matifali
matifali reviewed Feb 12, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@matifali matifali left a comment

Choose a reason for hiding this comment

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

a few more suggestions.

@johnstcn @matifali
Update scaletest/terraform/k8s/coder.tf
a22a186 
Co-authored-by: Muhammad Atif Ali <atif@coder.com>
@mafredri
Copy link
Copy Markdown
Member

mafredri commented Feb 12, 2024

We should then also remove startup_script_behaviour.

We could, but then we must rewrite all startup_script entries as coder_script resources with the matching blocking setting.

@johnstcn johnstcn merged commit 1cc51b0 into main Feb 12, 2024
@johnstcn johnstcn deleted the cj/rm-startup-script-timeout branch February 12, 2024 14:29
@github-actions github-actions Bot locked and limited conversation to collaborators Feb 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@matifali matifali matifali approved these changes

@mafredri mafredri mafredri approved these changes

@mtojek mtojek mtojek approved these changes

Assignees

@johnstcn johnstcn

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@johnstcn @mafredri @matifali @mtojek