Support vector search endpoints and indexes in DABs with direct deployment engine#4816
Support vector search endpoints and indexes in DABs with direct deployment engine#4816janniklasrose wants to merge 10 commits intomainfrom
Conversation
|
Commit: 396e5c3
22 interesting tests: 9 SKIP, 7 KNOWN, 6 flaky
Top 27 slowest tests (at least 2 minutes):
|
denik
left a comment
There was a problem hiding this comment.
Can you add invariant tests as well as more resource acceptance tests that recreate.
| api := m.GetMockVectorSearchEndpointsAPI() | ||
|
|
||
| t.Run("exists", func(t *testing.T) { | ||
| api.EXPECT(). |
There was a problem hiding this comment.
We prefer testserver over mocks. Let's drop this & if there is no acceptance test that covers this, let's add one.
| @@ -490,3 +490,32 @@ resources: | |||
| reason: immutable | |||
| - field: endpoint_id | |||
| reason: immutable | |||
|
|
|||
| vector_search_endpoints: | |||
There was a problem hiding this comment.
Did you run "make generate-direct"? I wonder if there is anything we can pull from API.
| "github.com/databricks/databricks-sdk-go/service/vectorsearch" | ||
| ) | ||
|
|
||
| func (s *FakeWorkspace) VectorSearchEndpointCreate(req Request) Response { |
There was a problem hiding this comment.
Let's keep the tradition of one resource per file, so vector_search_{endpoints,indexes}.go
| @@ -74,6 +74,13 @@ var knownMissingInRemoteType = map[string][]string{ | |||
| "pg_version", | |||
| "project_id", | |||
| }, | |||
| "vector_search_endpoints": { | |||
| "budget_policy_id", | |||
| "min_qps", | |||
There was a problem hiding this comment.
| resources.synced_database_tables.*.spec.timeseries_key string ALL | ||
| resources.synced_database_tables.*.unity_catalog_provisioning_state database.ProvisioningInfoState ALL | ||
| resources.synced_database_tables.*.url string INPUT | ||
| resources.vector_search_endpoints.*.budget_policy_id string INPUT STATE |
There was a problem hiding this comment.
Should we merge budget_policy_id and effective_budget_policy_id? So that we detect drift there? OR are there cases where effective_budget_policy_id is different due to some external policy?
|
Do these resources support permissions? |
| test_endpoint: | ||
| name: $ENDPOINT_NAME | ||
| endpoint_type: STANDARD | ||
| vector_search_indexes: |
There was a problem hiding this comment.
I would assume customers want to use JSON data to add it to the index, so we need to likely test and implement this
| if i.ID == "" { | ||
| return | ||
| } | ||
| baseURL.Path = "explore/vector-search/" + i.EndpointName + "/" + i.ID |
There was a problem hiding this comment.
This is a wrong URL, should be /api/2.0/vector-search/indexes/{index_name}
| workspace: | ||
| root_path: ~/.bundle/$UNIQUE_NAME | ||
|
|
||
| resources: |
There was a problem hiding this comment.
As a follow up, please add support for permissions for these resources
| MinQps: config.MinQps, | ||
| ForceSendFields: utils.FilterFields[vectorsearch.PatchEndpointRequest](config.ForceSendFields), | ||
| } | ||
| return r.client.VectorSearchEndpoints.PatchEndpoint(ctx, req) |
There was a problem hiding this comment.
You'll need to handle budget_policy_id changes here as well
| }) | ||
| } | ||
|
|
||
| // DoUpdate is intentionally not implemented. The Vector Search Index API has no |
There was a problem hiding this comment.
Shall we have an implementation and return and error if it's called, or what is the default behaviour?
cc @denik what's your thoughts on this?
There was a problem hiding this comment.
Omitting it is the right approach. This will return during planning if "update" action is planned.
Changes
Why
Tests