Add support for Vector Search Endpoint (direct only)#4887
Add support for Vector Search Endpoint (direct only)#4887janniklasrose wants to merge 38 commits intomainfrom
Conversation
andrewnester
left a comment
There was a problem hiding this comment.
Could you please add tests for bind / unbind for this resources before merging?
https://github.com/databricks/cli/tree/main/acceptance/bundle/deployment/bind
| } | ||
| } | ||
| info := remote.EndpointInfo | ||
| budgetPolicyId := info.EffectiveBudgetPolicyId // TODO: use info.BudgetPolicyId when available |
There was a problem hiding this comment.
Why this TODO though? Shouldn't we alway use EffectiveBudgetPolicyId anyway?
There was a problem hiding this comment.
EffectiveBudgetPolicyId is potentially unreachable by setting BudgetPolicyId (depending how backend resolves this given workspace policies etc), leading to permanent drift. Once the API (next SDK release) is declarative friendly, we can do what we do for other resources too (use remote .BudgetPolicyId or "" if not available)
| } | ||
|
|
||
| func (r *ResourceVectorSearchEndpoint) DoUpdate(ctx context.Context, id string, config *vectorsearch.CreateEndpoint, entry *PlanEntry) (*VectorSearchRefreshOutput, error) { | ||
| if entry.Changes.HasChange(pathBudgetPolicyId) { |
There was a problem hiding this comment.
Did you consider sending these two requests in parallel in separate goroutines? pros / cons
There was a problem hiding this comment.
Don't expect this to be much of a bottleneck, but they're independent so we could. There's precedent in model_serving_endpoint.go to make them sequential on purpose (though granted it's following Terraform there, which only does 1 call for vector search since they're missing the min_qps path).
There was a problem hiding this comment.
I will double-check with the VS team if the APIs are safe to call in parallel and update in a follow-up.
There was a problem hiding this comment.
No need to be blocked by this though, just an observation for later improvements if needed
| vector_search_endpoints: | ||
| foo: | ||
| name: test-endpoint-$UNIQUE_NAME | ||
| endpoint_type: STANDARD |
There was a problem hiding this comment.
Could you also add another config with permissions?
| trace $CLI vector-search-endpoints patch-endpoint "${endpoint_name}" --min-qps 5 | ||
|
|
||
| title "Plan detects drift and proposes update" | ||
| trace $CLI bundle plan |
There was a problem hiding this comment.
you can also add contains.py to encode expectation
trace $CLI bundle plan | contains.py "Plan: 0 to add, 0 to delete, 1 to update"
| } | ||
|
|
||
| // Vector search endpoints use the endpoint name as deployment id; the permissions API uses endpoint UUID. | ||
| if strings.HasPrefix(baseNode, "resources.vector_search_endpoints.") { |
There was a problem hiding this comment.
nit: the above if chain should be if/else chain.
|
|
||
| # Regenerate refschema for new resources | ||
| .PHONY: generate-refschema | ||
| generate-refschema: |
There was a problem hiding this comment.
can be skipped, you get it automatically when running test-update (unless you reference it in some skill / doc, but then it should be part of that PR).
| "description": "The alert_id is the canonical identifier of the alert.", | ||
| "$ref": "#/$defs/string" | ||
| "$ref": "#/$defs/string", | ||
| "x-since-version": "v0.296.0" |
There was a problem hiding this comment.
Seems unrelated to your changes, ideally should go to separate PR.
There was a problem hiding this comment.
Will rebase on SDK bump, which auto-gen'ed these
| "resources.synced_database_tables.*.spec.scheduling_policy": {"CONTINUOUS", "SNAPSHOT", "TRIGGERED"}, | ||
| "resources.synced_database_tables.*.unity_catalog_provisioning_state": {"ACTIVE", "DEGRADED", "DELETING", "FAILED", "PROVISIONING", "UPDATING"}, | ||
|
|
||
| "resources.vector_search_endpoints.*.endpoint_type": {"STANDARD"}, |
There was a problem hiding this comment.
Should not we have public preview one there? STORAGE_OPTIMIZED
https://docs.databricks.com/api/workspace/vectorsearchendpoints/createendpoint
There was a problem hiding this comment.
Waiting for SDK bump
| } | ||
|
|
||
| func (*ResourceVectorSearchEndpoint) RemapState(remote *VectorSearchRefreshOutput) *vectorsearch.CreateEndpoint { | ||
| if remote == nil || remote.EndpointInfo == nil { |
There was a problem hiding this comment.
The framework will not pass nil into any of the methods (RemapState, DoCreate etc), so we can drop nil checks from there.
|
|
||
| // VectorSearchRefreshOutput is remote state for a vector search endpoint. It embeds API response | ||
| // fields for drift comparison and adds endpoint_uuid for permissions; deployment state id remains the endpoint name. | ||
| type VectorSearchRefreshOutput struct { |
There was a problem hiding this comment.
Let's call this VectorSearchRemote? This is the most common convention:
~/work/cli/bundle/direct/dresources % git grep -w type | grep Remote
app.go:type AppRemote struct {
job.go:// JobRemote is the return type for DoRead. It embeds JobSettings so that all
job.go:type JobRemote struct {
model.go:type MlflowModelRemote struct {
pipeline.go:// PipelineRemote is the return type for DoRead. It embeds CreatePipeline so that all
pipeline.go:type PipelineRemote struct {
We'll fix this one later as well:
~/work/cli/bundle/direct/dresources % git grep -w type | grep RefreshOut
model_serving_endpoint.go:type RefreshOutput struct {
There was a problem hiding this comment.
Sounds good! Renamed single occurrence of RefreshOut in #4989
| _, err := r.client.VectorSearchEndpoints.PatchEndpoint(ctx, vectorsearch.PatchEndpointRequest{ | ||
| EndpointName: id, | ||
| MinQps: config.MinQps, | ||
| ForceSendFields: utils.FilterFields[vectorsearch.PatchEndpointRequest](config.ForceSendFields, "MinQps"), |
There was a problem hiding this comment.
if you're excluding MinQps here and there is nothing else, can we just set ForceSendFields to nil?
Changes
Adds
vector_search_endpointsas a first-class resource type, using the direct deployment engine (only, no TF support).New configuration surface
Required fields:
name,endpoint_type. Optional:min_qps,budget_policy_id,permissions.Key points to note
State ID = endpoint name. The CRUD API identifies endpoints by name; the UUID
(
endpoint_uuid) is stored separately in the refresh output for use by the permissions API.endpoint_typeis immutable. Changing it triggers delete + recreate (resources.yml).Two separate update APIs.
DoUpdatedispatches to:UpdateEndpointBudgetPolicywhenbudget_policy_idchangesPatchEndpointwhenmin_qpschangesThese can fire in the same deploy if both fields change.
budget_policy_iddrift is suppressed. The API returnseffective_budget_policy_id(which includes inherited workspace policies), not the user-set value. Until the SDK
exposes
budget_policy_idseparately, remote changes to this field are ignored(
reason: effective_vs_requestedinresources.yml). See TODO inbundle/direct/dresources/vector_search_endpoint.go:53.Permissions use UUID, not name. The
PreparePermissionsInputConfigfunction uses${...endpoint_uuid}as the object ID when constructing the permissions API path forvector search endpoints.
Direct-only validation.
ValidateDirectOnlyResources(bundle/config/mutator/) emitsan error at plan/deploy time if vector_search_endpoints are present in a non-direct bundle.
Vector Search Endpoints have no Terraform provider.
No dev-mode name prefix. Like UC resources, vector search endpoint names are NOT
prefixed with the dev user name in development mode.
Tests