-
Notifications
You must be signed in to change notification settings - Fork 962
feat: claim prebuilds based on workspace parameters instead of preset id #19279
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?
Changes from all commits
0890d84
a94d03e
96f0b37
a9e0f1d
292a1a9
b40e78f
6fe0dd0
735e937
151a039
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -245,3 +245,29 @@ INNER JOIN organizations o ON o.id = w.organization_id | |
WHERE NOT t.deleted AND wpb.build_number = 1 | ||
GROUP BY t.name, tvp.name, o.name | ||
ORDER BY t.name, tvp.name, o.name; | ||
|
||
-- name: FindMatchingPresetID :one | ||
-- FindMatchingPresetID finds a preset ID that is the largest exact subset of the provided parameters. | ||
-- It returns the preset ID if a match is found, or NULL if no match is found. | ||
-- The query finds presets where all preset parameters are present in the provided parameters, | ||
-- and returns the preset with the most parameters (largest subset). | ||
WITH provided_params AS ( | ||
SELECT unnest(@parameter_names::text[]) AS name, | ||
unnest(@parameter_values::text[]) AS value | ||
), | ||
preset_matches AS ( | ||
SELECT | ||
tvp.id AS template_version_preset_id, | ||
COALESCE(COUNT(tvpp.name), 0) AS total_preset_params, | ||
COALESCE(COUNT(pp.name), 0) AS matching_params | ||
FROM template_version_presets tvp | ||
LEFT JOIN template_version_preset_parameters tvpp ON tvpp.template_version_preset_id = tvp.id | ||
LEFT JOIN provided_params pp ON pp.name = tvpp.name AND pp.value = tvpp.value | ||
WHERE tvp.template_version_id = @template_version_id | ||
GROUP BY tvp.id | ||
) | ||
SELECT pm.template_version_preset_id | ||
FROM preset_matches pm | ||
WHERE pm.total_preset_params = pm.matching_params -- All preset parameters must match | ||
ORDER BY pm.total_preset_params DESC -- Return the preset with the most parameters | ||
LIMIT 1; | ||
Comment on lines
+249
to
+273
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So the logic here is that if we request a preset matching a small number of parameters, we will get the most specific preset? Would it make more sense to return the most general (smallest number of parameters)? |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
package prebuilds | ||
|
||
import ( | ||
"context" | ||
"database/sql" | ||
"errors" | ||
|
||
"github.com/google/uuid" | ||
"golang.org/x/xerrors" | ||
|
||
"github.com/coder/coder/v2/coderd/database" | ||
) | ||
|
||
// FindMatchingPresetID finds a preset ID that matches the provided parameters. | ||
// It returns the preset ID if a match is found, or uuid.Nil if no match is found. | ||
// The function performs a bidirectional comparison to ensure all parameters match exactly. | ||
func FindMatchingPresetID( | ||
ctx context.Context, | ||
store database.Store, | ||
templateVersionID uuid.UUID, | ||
parameterNames []string, | ||
parameterValues []string, | ||
) (uuid.UUID, error) { | ||
if len(parameterNames) != len(parameterValues) { | ||
return uuid.Nil, xerrors.New("parameter names and values must have the same length") | ||
} | ||
|
||
result, err := store.FindMatchingPresetID(ctx, database.FindMatchingPresetIDParams{ | ||
TemplateVersionID: templateVersionID, | ||
ParameterNames: parameterNames, | ||
ParameterValues: parameterValues, | ||
}) | ||
if err != nil { | ||
// Handle the case where no matching preset is found (no rows returned) | ||
if errors.Is(err, sql.ErrNoRows) { | ||
return uuid.Nil, nil | ||
} | ||
return uuid.Nil, xerrors.Errorf("find matching preset ID: %w", err) | ||
} | ||
|
||
return result, nil | ||
} |
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 like this, but I can't find a better way and there is precedent for this approach. Open to alternatives.
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.
The alternative is to essentially duplicate the logic in
GetTemplateVersionByID
, which I'm less a fan of.