-
Notifications
You must be signed in to change notification settings - Fork 30
[enhanced builds] add build_machine_type option to project creation #319
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
base: main
Are you sure you want to change the base?
Conversation
} | ||
|
||
type ResourceConfig struct { | ||
FunctionDefaultMemoryType *string `json:"functionDefaultMemoryType,omitempty"` | ||
FunctionDefaultTimeout *int64 `json:"functionDefaultTimeout,omitempty"` | ||
Fluid *bool `json:"fluid,omitempty"` | ||
ElasticConcurrencyEnabled *bool `json:"elasticConcurrencyEnabled,omitempty"` | ||
BuildMachineType *string `json:"buildMachineType,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
omitempty
here means we'll never actually send null
to the API. Probably want to remove that.
"build_machine_type": schema.StringAttribute{ | ||
Description: "When `on_demand_concurrent_builds` is enabled, choose the build machine: `standard` (4 vCPU, 8 GB) or `enhanced` (8 vCPU, 16GB)", | ||
Optional: true, | ||
Computed: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can add UseStateForUnknown
, as this setting will never change based on the other settings changing, right?
Or does disabling on_demand_concurrent_builds
affect this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
edit: i'm a fool, this is the data source.
@@ -393,6 +393,14 @@ For more detailed information, please see the [Vercel documentation](https://ver | |||
Optional: true, | |||
Computed: true, | |||
}, | |||
"build_machine_type": schema.StringAttribute{ | |||
Description: "When `on_demand_concurrent_builds` is enabled, choose the build machine: `standard` (4 vCPU, 8 GB) or `enhanced` (8 vCPU, 16GB)", | |||
Optional: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't be optional on the data source. Only computed. (it's a read-only property here).
Description: "When `on_demand_concurrent_builds` is enabled, choose the build machine: `standard` (4 vCPU, 8 GB) or `enhanced` (8 vCPU, 16GB)", | ||
Optional: true, | ||
Computed: true, | ||
Validators: []validator.String{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for validators on the data source.
Validators: []validator.String{ | ||
stringvalidator.OneOf("standard", "enhanced"), | ||
}, | ||
PlanModifiers: []planmodifier.String{stringplanmodifier.UseStateForUnknown()}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if on_demand_concurrent_builds
is disabled?
Description: "When `on_demand_concurrent_builds` is enabled, choose the build machine: `standard` (4 vCPU, 8 GB) or `enhanced` (8 vCPU, 16GB)", | ||
Optional: true, | ||
Computed: true, | ||
Validators: []validator.String{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's figure out how to add a validator to make sure on_demand_concurrent_builds
is enabled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have ModifyPlan
on this resource - we can do it there.
@@ -1101,6 +1111,11 @@ func (r *ResourceConfig) toClientResourceConfig(onDemandConcurrentBuilds types.B | |||
if !onDemandConcurrentBuilds.IsUnknown() { | |||
resourceConfig.ElasticConcurrencyEnabled = onDemandConcurrentBuilds.ValueBoolPointer() | |||
} | |||
if !buildMachineType.IsUnknown() && *resourceConfig.ElasticConcurrencyEnabled { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if resourceConfig.ElasticConcurrencyEnabled
is null
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need this logic here, we can just use a validator, and set buildMachineType
without worrying about its relationship to elastic-concurrency.
if !buildMachineType.IsUnknown() && *resourceConfig.ElasticConcurrencyEnabled { | ||
// `build_machine_type` can only be set when `on_demand_concurrent_builds` | ||
// is enabled. | ||
resourceConfig.BuildMachineType = buildMachineType.ValueStringPointer() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't forget to convert standard
to a null pointer?
@@ -1472,6 +1487,7 @@ func convertResponseToProject(ctx context.Context, response client.ProjectRespon | |||
ResourceConfig: resourceConfig, | |||
NodeVersion: types.StringValue(response.NodeVersion), | |||
OnDemandConcurrentBuilds: types.BoolValue(response.ResourceConfig.ElasticConcurrencyEnabled), | |||
BuildMachineType: types.StringValue(*response.ResourceConfig.BuildMachineType), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't forget to convert null
to standard
? Also, dereferencing the pointer he could panic.
After the release of on-demand enhanced builds, we are adding support for this feature at the project level. We are adding a new option,
build_machine_type
, to the project.Requisites of this feature:
on_demand_concurrent_builds
must be set to true. If it is set to false andbuild_machine_type
is set to something, we should error.on_demand_concurrent_builds
is set to true).standard
andenhanced
.Where does this feature reside?
It resides in
Project settings > Build and Deployment > On-Demand Concurrent Builds
CURL Examples
Setting the
build_machine_type
to enhanced:Setting the
build_machine_type
to standard (notice that we send null instead of standard):