feat: allow presets to define prebuilds #373
Conversation
| @@ -83,6 +91,11 @@ func workspaceDataSource() *schema.Resource { | |||
| Computed: true, | |||
| Description: "The access port of the Coder deployment provisioning this workspace.", | |||
| }, | |||
| "prebuild_count": { | |||
There was a problem hiding this comment.
is_prebuild & prebuild_count looks the same to me, what's the point to have both?
There was a problem hiding this comment.
This seems to be duplication, yes. We could probably get rid of "is_prebuild" and just check the count. Looking at the rest of the code, we are following the pattern that was set by the "transition" and "start_count" parameters. They have the same relationship.
I'm not sure whether to remove "is_prebuild" or keep it.
There was a problem hiding this comment.
I don't like neither prebuild_count nor start_count. But probably there is some reason we're using, I'm not aware of.
My guess is we're using start_count to map transition statuses, like:
- stop: 0
- start: 1
etc...
Also looks like there is some duplication here as well:
- start_count
- transition
But I don't know why it's needed.
We can ask original author of this approach with start_count, transition?
Maybe it was done in a rush, and there is no point to continue using this approach.
| prebuildCount = 1 | ||
| _ = rd.Set("is_prebuild", true) | ||
| } | ||
| _ = rd.Set("prebuild_count", prebuildCount) |
There was a problem hiding this comment.
nit: I'd rather write it like this:
if prebuild == "true" {
_ = rd.Set("is_prebuild", true)
_ = rd.Set("prebuild_count", 1)
} else {
_ = rd.Set("is_prebuild", false)
_ = rd.Set("prebuild_count", 0)
}I think it's more logical, but it's not a big deal
| @@ -121,3 +139,7 @@ func workspaceDataSource() *schema.Resource { | |||
| }, | |||
| } | |||
| } | |||
|
|
|||
| func IsPrebuildEnvironmentVariable() string { | |||
| return "CODER_WORKSPACE_IS_PREBUILD" | |||
There was a problem hiding this comment.
I guess it can be const instead of func, but up to you
| } | ||
|
|
||
| func workspacePresetDataSource() *schema.Resource { | ||
| return &schema.Resource{ | ||
| SchemaVersion: 1, | ||
|
|
||
| Description: "Use this data source to predefine common configurations for workspaces.", | ||
| Description: "Use this data source to predefine common configurations for coder workspaces. Users will have the option to select a defined preset, which will automatically apply the selected configuration. Any parameters defined in the preset will be applied to the workspace. Parameters that are not defined by the preset will still be configurable when creating a workspace.", | ||
| ReadContext: func(ctx context.Context, rd *schema.ResourceData, i interface{}) diag.Diagnostics { | ||
| var preset WorkspacePreset |
There was a problem hiding this comment.
@SasSwart btw: I just noticed code in the ReadContext is basically no-op?
It reads data and then forgets them.
Probably it's fine if main goal is to send this data downstream to provisioner
But still what is the purpose of this code?
- is it for validation purposes? But I'm not sure maybe validations happens earlier?
- is it for helping debugging in the future?
There was a problem hiding this comment.
Most of the decoding is defunct, yes. I've removed it.
I don't think we should rely on this for validation.
We define a schema below, which Terraform already uses for validation.
I've removed the defunct code here: #376
This PR closes #362