feat: add support for alter schema drop vector index - #1991
Conversation
There was a problem hiding this comment.
Orca Security Scan Summary
| Status | Check | Issues by priority | |
|---|---|---|---|
| Infrastructure as Code | View in Orca | ||
| SAST | View in Orca | ||
| Secrets | View in Orca | ||
| Vulnerabilities | View in Orca |
There was a problem hiding this comment.
Pull request overview
Adds client support for dropping a named vector’s index via the schema REST endpoint, mirroring the existing “delete property index” capability in the collections config API.
Changes:
- Added
delete_vector_index(vector_name)to the collection config executor, issuingDELETE /v1/schema/{className}/vectors/{vectorIndexName}/index. - Added sync and async type stubs for
delete_vector_index(...).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| weaviate/collections/config/executor.py | Implements the new delete-vector-index operation via the config executor. |
| weaviate/collections/config/sync.pyi | Exposes the new method in the sync config type stub. |
| weaviate/collections/config/async_.pyi | Exposes the new method in the async config type stub. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def resp(res: Response) -> bool: | ||
| return res.status_code == 200 | ||
|
|
There was a problem hiding this comment.
Keeping as-is for consistency — the sibling delete_property_index uses the identical callback pattern. Happy to simplify both together in a follow-up.
d728dd9 to
7e5cd7f
Compare
There was a problem hiding this comment.
Orca Security Scan Summary
| Status | Check | Issues by priority | |
|---|---|---|---|
| Secrets | View in Orca |
3657487 to
736a7cd
Compare
| NONE: The index of this vector has been dropped, see ``collection.config.delete_vector_index``. | ||
| The vector data is still stored, but it cannot be searched. This value is reported by the | ||
| server only, it cannot be used to configure a vector. | ||
| """ |
There was a problem hiding this comment.
Leaving as-is for now: no public Configure.VectorIndex.* factory emits NONE (only the internal create models reference the field), and the server rejects "none" on both create and update, so the exposure is constructing private classes directly. Can add a validator in a follow-up if users actually hit it.
7e5cd7f to
4e2a580
Compare
736a7cd to
be6c8dc
Compare
4e2a580 to
2dbfaed
Compare
|
thought: Do you think this PR could do with fewer tests? The new endpoint is very simple and it seems to me like one "happy path" test will catch most things. Other tests verify Weaviate's behavior more than client's logic, and some may add up to 30s of pipeline time. |
There was a problem hiding this comment.
Orca Security Scan Summary
| Status | Check | Issues by priority | |
|---|---|---|---|
| Infrastructure as Code | View in Orca | ||
| SAST | View in Orca | ||
| Secrets | View in Orca | ||
| Vulnerabilities | View in Orca |
|
@bevzzz fair point — slimmed in b92dbb7, keeping the coverage but moving it down the pyramid: the integration test is now a single happy-path journey (create → drop → poll → sibling index intact + searchable). The unknown-name case moved to the mock suite (422 → |
|
@copilot review |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1991 +/- ##
==========================================
+ Coverage 86.64% 88.35% +1.71%
==========================================
Files 300 302 +2
Lines 23172 23384 +212
==========================================
+ Hits 20077 20661 +584
+ Misses 3095 2723 -372 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Follow-up to the `delete_vector_index` support, addressing review findings. After a successful drop, Weaviate keeps the vector in the schema as `vectorIndexType: "none"` with no `vectorIndexConfig`. The client asserted that every named vector has an index config, so `collection.config.get()` and `client.collections.list_all()` raised `AssertionError` for every collection in the cluster once any vector index had been dropped. `_NamedVectorConfig. vector_index_config` is now optional, `VectorIndexType` gained a server-reported `NONE` member and `to_dict()` round-trips it. `collection.config.update()` on a dropped vector raised a bare `KeyError: 'vectorIndexConfig'` from the schema merge. Both the current and the deprecated merge paths now go through one helper that raises a `WeaviateInvalidInputError` explaining that a dropped index cannot be re-created. The docstring claimed the index could be regenerated and that a missing vector raises `WeaviateInvalidInputError`. Neither is true: Weaviate rejects re-creating a dropped index, and an unknown vector name comes back as a 422. It now also documents that the endpoint is experimental and needs `ENABLE_EXPERIMENTAL_ALTER_SCHEMA_DROP_VECTOR_INDEX_ENDPOINT=true`, that only named vectors can be dropped, and that the drop is applied asynchronously. The error message no longer blames a missing vector for what is usually a disabled endpoint. Tests: unit coverage for parsing, exporting and updating a dropped vector, mock coverage for the request path and the disabled-endpoint response, and integration coverage gated at 1.39.0. The CI compose file enables the experimental endpoint; that flag can be dropped once 1.39.0 is GA. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Replace the assert on dropped-vector schema shape with an explicit SchemaValidationError so a named vector missing vectorIndexConfig fails fast even under python -O; pinned by a new parser unit test - Slim the integration test to the happy path: the unknown-name error contract moved to the mock suite (422 -> UnexpectedStatusCodeError) and the redundant list_all/invalid-input assertions are covered by the existing unit and mock tests Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A single-vector (non-named) collection whose vector index is dropped with `collection.config.delete_vector_index` comes back from the server with no top-level `vectorizer` (and no `vectorConfig`, `vectorIndexType` or `vectorIndexConfig`). `__get_vectorizer` accessed `schema["vectorizer"]` unguarded and raised `KeyError: 'vectorizer'`, so both `config.get()` and `collections.list_all()` crashed on such a collection. Return `None` when the key is absent, matching how a dropped named vector yields `vector_index_config is None`. Add parser tests for both entry points. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
9d7b916 to
ccad4cc
Compare
Summary
delete_vector_index(vector_name)method to collection config, allowing users to drop a named vector's index viaDELETE /v1/schema/{className}/vectors/{vectorIndexName}/indexdelete_property_indexmethodCloses #1990