Skip to content

feat(config): add runtime property reindex support (Weaviate v1.39 GA API) - #2098

Open
g-despot wants to merge 16 commits into
dev/1.39from
feat/runtime-property-reindex
Open

feat(config): add runtime property reindex support (Weaviate v1.39 GA API)#2098
g-despot wants to merge 16 commits into
dev/1.39from
feat/runtime-property-reindex

Conversation

@g-despot

@g-despot g-despot commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

Closes #2097

Adds client support for the Runtime Property Reindex GA API (Weaviate v1.39, now released), per the RFC linked from #2097. Targets dev/1.39.

New API (sync + async, on collection.config)

  • update_property_index(property_name, index_name: InvertedIndexType, *, tokenization=None, algorithm=None, tenants=None, wait_for_completion=False, timeout=None): declarative upsert (create or migrate an index). Returns InvertedIndexTask (202 STARTED with task id, or 200 NO_OP); with wait_for_completion=True waits for the reindex to complete and returns the final InvertedIndexStatus.
  • rebuild_property_index(property_name, index_name, *, tenants=None, wait_for_completion=False, timeout=None): same-config rebuild.
  • cancel_property_index_task(property_name, index_name): cancels the in-flight reindex task (CANCELLED or NO_OP, idempotent).
  • get_property_indexes(): per-property, per-index status listing (CollectionInvertedIndexes), including migration progress, task ids, and in-flight target tokenization/algorithm.
  • delete_property_index(property_name, index_name): unchanged v1.36 behavior (see the deprecation note below).

index_name on the new methods takes the InvertedIndexType enum (SEARCHABLE, FILTERABLE, RANGE_FILTERS); algorithm takes the new BM25Algorithm enum (BLOCKMAX is the only valid target). New read-side types (InvertedIndexStatus with a .state: InvertedIndexState, InvertedIndexTask, PropertyInvertedIndexes, CollectionInvertedIndexes) and enums are exported from weaviate.classes.config / weaviate.outputs.config. New exceptions: ReindexFailedError, ReindexCanceledError, ReindexTimeoutError. All new methods are gated on server >= 1.39.0.

Behavior change to an existing API (deprecation)

delete_property_index (GA since v1.36) still accepts a raw string index_name, but doing so now emits a DeprecationWarning (Dep030): pass an InvertedIndexType instead. The value still works; only a warning is added.

wait_for_completion semantics

wait_for_completion=True polls GET /v1/schema/{class}/indexes (the collection-scoped, namespace-transparent status endpoint) until the targeted index reaches ready with the requested configuration. failed/cancelled raise ReindexFailedError/ReindexCanceledError. timeout (seconds, default None) bounds the wait; even with None, a stalled or vanished index entry is bounded by an internal no-progress guard (raising ReindexTimeoutError) rather than hanging, while a legitimately long reindex is never cut off. A rebuild has no observable end-state on the status projection, so its wait is best-effort.

Testing

  • 107 mock tests over the /v1/schema/{class}/indexes wire: routes and bodies for all endpoints, InvertedIndexType/BM25Algorithm enum handling (incl. wire-value serialization and the WAND-rejected guard), 200 NO_OP vs 202, tolerant status/algorithm parsing, the Dep030 deprecation, and the full wait matrix: convergence, stale-pre-flip not accepted, finalize-window (indexing@1.0) not treated as done, failed/cancelled, timeout, the no-progress/stall bound, and a proof that a long-running reindex is not cut off.
  • Integration tests (sync + async, skipped below server 1.39): searchable lifecycle, rangeFilters creation, coupled tokenization change, multi-tenant scoping, in-flight cancel.
  • Verified live against semitechnologies/weaviate:1.39.0, including the real stale-pre-flip race (ready(word) -> indexing(target=field) -> ready(field)), which the wait correctly skips.

g-despot added 3 commits July 19, 2026 21:49
Adds collection.config methods for the GA runtime property reindex API:
- update_property_index: declarative create-or-migrate upsert (PUT
  /schema/{class}/properties/{prop}/index/{indexType}) with optional
  wait_for_completion polling of the index status endpoint
- rebuild_property_index: rebuild an existing index from scratch, with
  tenant selection and optional wait_for_completion
- cancel_property_index_task: idempotent cancellation of a live task
- get_property_indexes: parse GET /schema/{class}/indexes into new
  CollectionPropertyIndexes/PropertyIndexStatus read-side types

All methods raise WeaviateUnsupportedFeatureError below server 1.39.0.
Mock tests exercise the exact REST routes against a 1.39-advertising mock
server (upsert 202/NO_OP, rebuild, cancel CANCELLED/NO_OP, tenants csv
encoding, status parsing incl. coupled task entries) and assert
WeaviateUnsupportedFeatureError against a 1.36 mock. Integration tests
cover the searchable lifecycle, rangeFilters creation, coupled
tokenization changes, multi-tenant selection and the async client,
skipping below server 1.39.0.
- accept a bare string for the tenants argument of update_property_index
  and rebuild_property_index, normalizing it to a single-element list so
  it cannot be exploded into a per-character csv (export API precedent)
- override to_dict on _PropertyIndexes/_CollectionPropertyIndexes so the
  nested dataclass lists serialize to JSON-compatible dicts
  (_CollectionConfig precedent)
- drop the dead list branch for dataType in the index status parser
- cover the ReindexFailedError/ReindexCanceledError wait paths of both
  update and rebuild, pin the empty request body of the rebuild/cancel
  mocks, and prove json.dumps(...to_dict()) round-trips

@orca-security-eu orca-security-eu Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Orca Security Scan Summary

Status Check Issues by priority
Passed Passed Infrastructure as Code high 0   medium 0   low 0   info 0 View in Orca
Passed Passed SAST high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Secrets high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Vulnerabilities high 0   medium 0   low 0   info 0 View in Orca

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds client support for Weaviate v1.39’s Runtime Property Reindex GA API by introducing new collection-config methods (sync + async), output models, and errors, plus mock and integration test coverage for the new endpoints and wait/poll behavior.

Changes:

  • Add update_property_index, rebuild_property_index, cancel_property_index_task, and get_property_indexes to the collection config executor, including wait_for_completion polling and version gating (>= 1.39.0).
  • Introduce new output dataclasses/enums for property index tasks/status listings and export them via weaviate.outputs.config, plus new terminal exceptions for failed/canceled reindex tasks.
  • Add mock tests and integration tests covering routes/bodies, parsing, tenants encoding, version gating, and wait-path terminal failures.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
weaviate/outputs/config.py Re-exports new property-index output types for public consumption.
weaviate/exceptions.py Adds new exceptions for terminal reindex outcomes (failed/canceled).
weaviate/collections/config/sync.pyi Adds typed sync stubs for the new property-index config methods (incl. overloads).
weaviate/collections/config/async_.pyi Adds typed async stubs for the new property-index config methods (incl. overloads).
weaviate/collections/config/executor.py Implements the new runtime property index endpoints + polling logic + version gating.
weaviate/collections/classes/config.py Adds enums/dataclasses representing property-index task and status payloads.
weaviate/collections/classes/config_methods.py Adds JSON->dataclass parsing helpers for the new property-index responses.
mock_tests/test_property_reindex.py Adds mock tests for request/response shapes, tenants encoding, version gating, and wait-path errors.
integration/test_collection_config.py Adds integration tests for the searchable/rangeFilters lifecycle and async variants (skipping <1.39).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread weaviate/collections/config/executor.py Outdated
Comment thread weaviate/collections/config/executor.py Outdated
@codecov-commenter

codecov-commenter commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.28682% with 21 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (dev/1.39@d38bc5b). Learn more about missing BASE report.

Files with missing lines Patch % Lines
integration/test_collection_config.py 90.26% 11 Missing ⚠️
weaviate/collections/config/executor.py 95.69% 8 Missing ⚠️
weaviate/collections/classes/config_methods.py 92.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             dev/1.39    #2098   +/-   ##
===========================================
  Coverage            ?   88.60%           
===========================================
  Files               ?      304           
  Lines               ?    24093           
  Branches            ?        0           
===========================================
  Hits                ?    21347           
  Misses              ?     2746           
  Partials            ?        0           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Validate the tenants argument of update_property_index and
rebuild_property_index as str | List[str] | None before the csv join so
invalid input raises WeaviateInvalidInputError instead of a raw
TypeError, matching the library's _validate_input idiom already applied
to property_name/index_name. Document the WeaviateInvalidInputError in
the affected docstrings and cover the validation with mock tests.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Comment thread weaviate/collections/classes/config.py
Comment thread weaviate/collections/classes/config_methods.py
g-despot added 6 commits July 21, 2026 09:31
The static delete_property_index error prefix asserted a single cause
("Property may not exist") but the DELETE 422 also covers an invalid
index type and the in-flight-reindex mutation guard, whose server
message the prefix contradicted. Name the failed operation instead and
let the appended response body carry the cause, matching the file's
phrasing style. Pin the behavior with a mock test surfacing a
server-style 422 in-flight-reindex message.
Adds a PropertyIndexType str-enum (SEARCHABLE, FILTERABLE,
RANGE_FILTERS) accepted alongside the IndexName literals on
update_property_index, rebuild_property_index,
cancel_property_index_task and delete_property_index (backward-
compatible widening of the v1.36 signature). The value is normalized to
its wire form at the top of each method so paths, params and error
messages never render the enum repr. Route-equality mock tests pin that
the enum and literal forms hit identical routes, incl. RANGE_FILTERS ->
rangeFilters. Read-side status types keep plain literals/str for
forward compatibility.
Documents on update_property_index that a tokenization change via the
searchable index also retokenizes an existing filterable index as one
coupled task (shared taskId) and thereby changes filter matching, with
filterable as the target for bucket-only changes and cancellation
applying to the whole task. Explains why PropertyIndexes.data_type is a
plain str (primitives match the DataType enum, references carry the
target collection name) and pins reference-property parsing with a mock
test.
The RANGE_FILTERS docstring said "rangeable", the internal write-path
alias the RFC deliberately keeps unsurfaced — say rangeFilters instead.
Extends the route-equality parametrization with FILTERABLE and adds
enum-vs-literal route cases for rebuild_property_index and
cancel_property_index_task.
Renames the unreleased public types PropertyIndexType/State/Status/
Task/TaskStatus to InvertedIndexType/State/Status/Task/TaskStatus and
PropertyIndexes/CollectionPropertyIndexes to PropertyInvertedIndexes/
CollectionInvertedIndexes, incl. private counterparts and parser names.
The runtime reindex API only ever touches inverted indexes; a future
vector reindex must not collide with these names. Method names, the
released IndexName alias and the deliberately generic Reindex*Error
exceptions are unchanged.
Tightens index_name to InvertedIndexType only on update_property_index,
rebuild_property_index and cancel_property_index_task (all overloads
and impls); delete_property_index keeps accepting the IndexName
literals as released v1.36 API. Runtime leniency is preserved — the
value is still normalized and validated as a str, so raw strings keep
working and keep hitting the same routes (pinned by the literal legs of
the route-equality mock tests, which sit outside the pyright scope).
g-despot added 2 commits July 29, 2026 10:49
The CI pin 1.39.0-rc.0-b41225e was a stale mid-review build of core
#12252 that emitted an IN_PROGRESS task status which never shipped; pin
1.39.0-rc.1 instead. Parse the task status tolerantly — known values
map to InvertedIndexTaskStatus, unknown values pass through as plain
strings, since the spec declares the field an open-vocabulary string —
and pin that with a mock test. Upgrade the coupled-tokenization test to
the final join contract: an identical re-PUT while the task is in
flight returns 202 with the existing taskId and STARTED, verified live
against 1.39.0-rc.1 (post-completion re-PUT stays 200 NO_OP).
@g-despot
g-despot requested a review from Copilot July 29, 2026 12:20
@g-despot
g-despot marked this pull request as ready for review July 29, 2026 12:20
@g-despot
g-despot requested a review from a team as a code owner July 29, 2026 12:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (8)

weaviate/collections/config/sync.pyi:125

  • rebuild_property_index is currently typed to only accept InvertedIndexType, but the runtime accepts the canonical IndexName literals too. Widen index_name to Union[InvertedIndexType, IndexName] for both overloads to match the supported API surface.
    def rebuild_property_index(
        self,
        property_name: str,
        index_name: InvertedIndexType,
        *,

weaviate/collections/config/async_.pyi:127

  • rebuild_property_index is currently typed to only accept InvertedIndexType, but the runtime accepts canonical IndexName literals too. Widen index_name to Union[InvertedIndexType, IndexName] for both overloads to match the supported API surface.
    async def rebuild_property_index(
        self,
        property_name: str,
        index_name: InvertedIndexType,
        *,

weaviate/collections/config/async_.pyi:142

  • cancel_property_index_task is currently typed to only accept InvertedIndexType, but the runtime accepts canonical IndexName literals as well. Widen index_name to Union[InvertedIndexType, IndexName] to match runtime behavior and avoid forcing # type: ignore at call sites.
    async def cancel_property_index_task(
        self, property_name: str, index_name: InvertedIndexType
    ) -> InvertedIndexTask: ...

weaviate/collections/config/sync.pyi:140

  • cancel_property_index_task is currently typed to only accept InvertedIndexType, but the runtime accepts canonical IndexName literals as well. Widen index_name to Union[InvertedIndexType, IndexName] to match the supported API and avoid # type: ignore at call sites.
    def cancel_property_index_task(
        self, property_name: str, index_name: InvertedIndexType
    ) -> InvertedIndexTask: ...

weaviate/collections/config/executor.py:781

  • update_property_index/rebuild_property_index/cancel_property_index_task accept IndexName literals at runtime (see the else index_name cast), but the public type signature here restricts index_name to InvertedIndexType. This forces callers using the canonical string literals (e.g. "searchable") to use # type: ignore, and is inconsistent with delete_property_index, which already advertises Union[InvertedIndexType, IndexName]. Consider widening these signatures (and their overloads + the .pyi stubs) to Union[InvertedIndexType, IndexName] so the declared API matches the supported runtime behavior.
    def update_property_index(
        self,
        property_name: str,
        index_name: InvertedIndexType,
        *,

weaviate/collections/config/sync.pyi:119

  • update_property_index is currently typed to only accept InvertedIndexType, but the API is also invoked throughout the repo with canonical IndexName literals (e.g. "searchable"). Align the stub with delete_property_index and the runtime behavior by allowing Union[InvertedIndexType, IndexName] for index_name in both overloads.

This issue also appears in the following locations of the same file:

  • line 121
  • line 138
    def update_property_index(
        self,
        property_name: str,
        index_name: InvertedIndexType,
        *,

weaviate/collections/config/async_.pyi:121

  • update_property_index is currently typed to only accept InvertedIndexType, but the API is also used with canonical IndexName literals (e.g. "searchable"). Align the async stub with delete_property_index and the runtime behavior by allowing Union[InvertedIndexType, IndexName] for index_name in both overloads.

This issue also appears in the following locations of the same file:

  • line 123
  • line 140
    async def update_property_index(
        self,
        property_name: str,
        index_name: InvertedIndexType,
        *,

weaviate/outputs/config.py:10

  • The PR description and the implemented/exported naming don’t match: the description refers to PropertyIndexTask/PropertyIndexStatus/CollectionPropertyIndexes, but this PR exports InvertedIndexTask/InvertedIndexStatus/CollectionInvertedIndexes (and the executor/stubs/tests use those names). Please update the PR description (and any user-facing docs/release notes) to reflect the actual public API names, or rename consistently if the PropertyIndex* naming is still intended.
    CollectionInvertedIndexes,
    GenerativeConfig,
    GenerativeSearches,
    InvertedIndexConfig,
    InvertedIndexState,

@dirkkul
dirkkul changed the base branch from main to dev/1.39 July 31, 2026 08:07
Comment thread weaviate/collections/config/executor.py Outdated
Comment on lines +730 to +751
def __wait_for_property_index(
self, property_name: str, index_name: IndexName
) -> executor.Result[InvertedIndexStatus]:
if isinstance(self._connection, ConnectionAsync):

async def _execute() -> InvertedIndexStatus:
while True:
indexes = await executor.aresult(self.get_property_indexes())
entry = _find_property_index_status(indexes, property_name, index_name)
done = _terminal_property_index_status(entry, property_name, index_name)
if done is not None:
return done
await asyncio.sleep(1)

return _execute()
while True:
indexes = executor.result(self.get_property_indexes())
entry = _find_property_index_status(indexes, property_name, index_name)
done = _terminal_property_index_status(entry, property_name, index_name)
if done is not None:
return done
time.sleep(1)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correctness — wait_for_completion=True can return before the task even starts.

This polls /indexes and treats the first ready entry it sees as completion, but it never checks that the entry belongs to the task that was just submitted. task.task_id is parsed at line 868 / 886 and then discarded.

Concretely: rebuild_property_index("name", SEARCHABLE, wait_for_completion=True) on an already-ready index → POST .../rebuild returns 202 STARTED → the first GET /indexes fires before the server has flipped the entry to pending → the client returns the old ready status and reports the rebuild as done. Same hazard for a tokenization migration on an existing index.

The fix is to thread the submitted task_id into this helper and only accept an entry as terminal when entry.task_id matches it (or when the submission came back NO_OP, where returning the current state is correct). The coupled searchable+filterable case still works, since both entries share one taskId.

Minor, same helper: if the entry never appears at all, _find_property_index_status returns None forever and this spins with no timeout. Backup/export have no timeout either so it's consistent — but they don't have an "entry missing" path.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, the wait helper now uses the submitted task_id, also test added

Comment thread weaviate/collections/config/executor.py Outdated
Comment on lines +113 to +120
if entry.status == InvertedIndexState.FAILED:
raise ReindexFailedError(
f"Reindexing the '{index_name}' index of property '{property_name}' failed."
)
if entry.status == InvertedIndexState.CANCELLED:
raise ReindexCanceledError(
f"Reindexing the '{index_name}' index of property '{property_name}' was cancelled."
)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correctness (same root cause as the wait-loop comment). These raise on any failed/cancelled entry, regardless of which task produced it. A stale terminal state left behind by a previous reindex makes a fresh, healthy wait_for_completion=True blow up immediately.

Gating on the submitted task_id fixes this and the stale-ready race together.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, failed/cancelled now happen only when task_id matches

Note: This method is a declarative upsert operation. If the index does not exist, it is created
with the requested configuration. If it exists, it is migrated towards the requested
configuration. If the configuration already matches, no work is submitted and the returned
task reports a `NO_OP` status. The server accepts at most one configuration change per request.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docstring, the client, and the tests disagree here.

This says the server accepts at most one configuration change per request, but nothing client-side enforces it — and mock_tests/test_property_reindex.py:57 pins sending tokenization and algorithm in one PUT and asserts a 202 STARTED. One of the three is wrong.

Same gap for the two constraints documented just below: "tokenization … Not valid for rangeFilters" and "tenants … Only valid when creating a rangeFilters index on a multi-tenant collection". Both are documented, neither is validated.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, the test now uses only one update, the enforcement is only server-side

Comment thread mock_tests/test_property_reindex.py Outdated
weaviate_139_mock.expect_request(
f"{SCHEMA_PATH}/properties/name/index/searchable",
method="PUT",
json={"tokenization": "word", "algorithm": "blockmax"},

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pins tokenization + algorithm in a single PUT as valid (202 STARTED), but update_property_index's docstring says "The server accepts at most one configuration change per request" (executor.py:792). Either the docstring is wrong, or this test is pinning behaviour the server will reject.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, split into tokenization-only and algorithm-only tests

self,
property_name: str,
index_name: IndexName,
index_name: Union[InvertedIndexType, IndexName],

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API consistency — two public spellings for one concept, applied unevenly.

IndexName (Literal alias, public since 4.20.0) and the new InvertedIndexType enum carry the same three values. This method accepts Union[InvertedIndexType, IndexName]; the four new methods accept InvertedIndexType only — yet all five accept raw strings at runtime, and the tests deliberately pin that (test_*_enum_and_literal_hit_same_route, with # type: ignore).

Net effect: config.delete_property_index("name", "searchable") type-checks and config.rebuild_property_index("name", "searchable") doesn't, for no user-visible reason.

Suggest widening the four new signatures to the same union — non-breaking, matches what the runtime already does, and removes the # type: ignores from your own tests. (The alternative, deprecating IndexName, is fine too — just pick one.)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, keeping the new methods enum arguments and deprecating the string spelling instead

Comment thread weaviate/collections/config/executor.py Outdated
Comment on lines +828 to +843
_validate_input(
[_ValidateArgument(expected=[str], name="property_name", value=property_name)]
)
_validate_input([_ValidateArgument(expected=[str], name="index_name", value=index)])
_validate_input(
[_ValidateArgument(expected=[str, List[str], None], name="tenants", value=tenants)]
)

path = self.__property_index_path(property_name, index)
body: Dict[str, Any] = {}
if tokenization is not None:
body["tokenization"] = (
tokenization.value if isinstance(tokenization, Tokenization) else tokenization
)
if algorithm is not None:
body["algorithm"] = algorithm

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uneven validation. property_name, index_name and tenants go through _validate_input, but tokenization, algorithm and wait_for_completion don't — they're passed straight through to the body. export/executor.py:286 type-checks wait_for_completion as a bool, which is the closest precedent.

Also: tenants=[] passes validation (all(... for val in []) is True) and ends up sending ?tenants= with an empty value. Probably wants to be rejected, or normalised to None.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, added _validate_input for wait_for_completion

path=path,
weaviate_object={},
error_msg="Property index task may not have been cancelled.",
status_codes=_ExpectedStatusCodes(ok_in=[202], error="Cancel property index task"),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Status-code asymmetry. update_property_index accepts [200, 202]; this and rebuild_property_index accept [202] only.

Since a NO_OP update is documented to come back 200, worth confirming against the spec that a NO_OP cancel can't — otherwise the documented idempotent no-op path raises UnexpectedStatusCodeError.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed against the server, cancel always returns 202 (CANCELLED or NO_OP). Added a comment noting the NO_OP-is-also-202 contract.

Comment on lines +29 to +48
@pytest.fixture(scope="function")
def weaviate_139_mock(ready_mock: HTTPServer) -> Generator[HTTPServer, None, None]:
"""A mock server advertising Weaviate 1.39.0, which supports runtime property reindexing."""
ready_mock.expect_request("/v1/meta").respond_with_json({"version": "1.39.0"})
ready_mock.expect_request("/v1/nodes").respond_with_json(
{"nodes": [{"gitHash": "ABC", "status": "HEALTHY"}]}
)
ready_mock.expect_request("/v1/.well-known/openid-configuration").respond_with_response(
Response(json.dumps({}), status=404)
)
yield ready_mock


@pytest.fixture(scope="function")
def client_139(
weaviate_139_mock: HTTPServer, start_grpc_server: grpc.Server
) -> Generator[weaviate.WeaviateClient, None, None]:
client = weaviate.connect_to_local(port=MOCK_PORT, host=MOCK_IP, grpc_port=MOCK_PORT_GRPC)
yield client
client.close()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test gap — this file is entirely sync. The async branches of update_property_index and rebuild_property_index are separately-written duplicate code (the if isinstance(self._connection, ConnectionAsync) forks), and they're currently exercised only by test_property_reindex_async, which skips on any server below 1.39.

A weaviate.use_async_with_local variant of this fixture plus a couple of @pytest.mark.asyncio tests would cover the fork unconditionally.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, added use_async_with_local mock tests

Comment thread mock_tests/test_property_reindex.py Outdated
Comment on lines +115 to +126
weaviate_139_mock.expect_request(f"{SCHEMA_PATH}/indexes", method="GET").respond_with_json(
{
"collection": COLLECTION,
"properties": [
{
"name": "name",
"dataType": "text",
"indexes": [{"type": "searchable", "status": "ready", "tokenization": "word"}],
}
],
}
)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test gap — no multi-poll transition is ever tested. Every wait_for_completion test returns a terminal state on the first GET /indexes, so the loop body and the sleep in __wait_for_property_index are never exercised.

That's exactly why the stale-ready race (see the comment on executor.py:730) is invisible here. A pending → indexing → ready sequence — pytest-httpserver supports ordered handlers via expect_ordered_request — would cover the loop and regress the task_id fix.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, added an pending → indexing → ready test

assert status.status == InvertedIndexState.READY

# cancelling when no task is live is an idempotent no-op
task = collection.config.cancel_property_index_task("name", InvertedIndexType.SEARCHABLE)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test gap — only the idempotent NO_OP cancel path is covered end-to-end. An actual CANCELLED result is mock-only, so the one branch that does real work on the server is unproven.

Cancelling the in-flight coupled task from test_property_reindex_coupled_tokenization_change (which inserts 100 objects specifically so the task stays live) would exercise it, and would also cover ReindexCanceledError on the wait path.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, added an in-flight cancel to the tokenization test

- wait_for_completion: gate terminal acceptance on the submitted task_id so a
  stale ready/failed/cancelled entry left by a prior reindex can no longer be
  mistaken for this task completing (fixes premature return and spurious raise)
- extract shared __submit_property_index_task for update/rebuild; point
  delete_property_index at the __property_index_path helper
- deprecate string index_name: emit Dep030 when a raw string is passed to
  delete_property_index; the reindex methods take InvertedIndexType
- InvertedIndexStatus.type is now the InvertedIndexType enum; parse the status
  field tolerantly so an unknown state cannot crash the poll loop
- validate wait_for_completion; normalize an empty tenants list to no param
- tests: async mock coverage, a multi-poll transition regression test, a real
  in-flight CANCELLED e2e, and split the one-change-per-request PUT test

@orca-security-eu orca-security-eu Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Orca Security Scan Summary

Status Check Issues by priority
Passed Passed Secrets high 0   medium 0   low 0   info 0 View in Orca

…m enum (#2098)

- rename the poll-entry lifecycle field InvertedIndexStatus.status to .state so the field name matches its InvertedIndexState enum and no longer collides with InvertedIndexTask.status (review feedback)
- add BM25Algorithm(str, BaseEnum) {wand, blockmax}; update_property_index takes algorithm: Optional[BM25Algorithm] (serialized to the wire value), and the status entry's algorithm/target_algorithm parse tolerantly into it
…erty-reindex

# Conflicts:
#	.github/workflows/main.yaml

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (3)

weaviate/collections/config/executor.py:790

  • Same issue as the async branch: using assert entry is not None for control flow can raise AssertionError, and under python -O this could return None. Raise a library exception instead when the status entry cannot be found.
            indexes = executor.result(self.get_property_indexes())
            entry = _find_property_index_status(indexes, property_name, index_name)
            assert entry is not None
            return entry

weaviate/collections/config/executor.py:959

  • These new property-reindex methods are documented/typed as taking index_name: InvertedIndexType, but the implementation still accepts raw strings via the else index_name branch (same pattern appears in rebuild_property_index and cancel_property_index_task). This contradicts the PR description that only delete_property_index accepts legacy string literals, and it also means callers can keep using strings without any Dep030 warning.

Consider either (a) enforcing InvertedIndexType at runtime (raise WeaviateInvalidInputError for strings), or (b) officially accepting Union[InvertedIndexType, IndexName] here too and emitting the same Dep030 deprecation warning + updating stubs/docstrings/tests accordingly.

        self.__check_property_reindex_support("Collection config update_property_index")
        index = cast(
            IndexName,
            index_name.value if isinstance(index_name, InvertedIndexType) else index_name,
        )

weaviate/collections/config/executor.py:773

  • assert entry is not None is used for control flow in the wait-for-completion path. This can surface as an AssertionError in normal runs, and in python -O asserts are stripped so this branch could return None despite the non-optional return type. Prefer raising a library exception with a clear message when the expected status entry is missing.

This issue also appears on line 787 of the same file.

                if task_id is None:
                    indexes = await executor.aresult(self.get_property_indexes())
                    entry = _find_property_index_status(indexes, property_name, index_name)
                    assert entry is not None
                    return entry

…nded (#2098)

Reimplement wait_for_completion to poll GET /v1/schema/{class}/indexes (the
collection-scoped, namespace-transparent status endpoint) instead of a signal
that could hang forever:

- completion is the targeted index reaching `ready` with the requested config;
  config-match cleanly separates a real completion from a stale pre-flip `ready`,
  and `failed`/`cancelled` are matched by the namespace-clean taskId and raise
- add a `timeout` knob (ReindexTimeoutError) plus a no-progress guard, so a
  server-side fault (a vanished or stalled index entry) is bounded rather than
  hanging, while a legitimately long reindex is never cut off
- reject algorithm=WAND client-side (never a valid target), validate the
  tokenization/algorithm inputs, and raise a clear error instead of a bare
  assert when a NO_OP response has no matching index entry

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Suppressed comments (5)

weaviate/classes/config.py:45

  • If these types are meant to be part of the public surface of weaviate.classes.config, they should also be added to __all__ (this module consistently enumerates its exports there).
    "ReplicationDeletionStrategy",
    "Property",
    "InvertedIndexType",
    "PQEncoderDistribution",
    "PQEncoderType",

weaviate/collections/config/executor.py:852

  • stalled() only treats PENDING/INDEXING (or our task id) as “progress”. Because InvertedIndexStatus.state is Union[InvertedIndexState, str] (unknown server values pass through), an in-flight index with a new/unknown non-ready state will be counted as “no progress” and can incorrectly trigger the stall guard (ReindexTimeoutError). Consider treating unknown non-ready states as progress when the entry indicates it is in-flight (e.g. has progress or a target config).
            if entry is not None and (
                seen or entry.state in (InvertedIndexState.PENDING, InvertedIndexState.INDEXING)
            ):
                return False
            if entry is not None and is_rebuild and entry.state == InvertedIndexState.READY:

weaviate/collections/config/async_.pyi:100

  • Same as sync: model the deprecated string overload explicitly so type-checkers can warn on it while still allowing existing code.
    async def delete_property_index(
        self, property_name: str, index_name: Union[InvertedIndexType, IndexName]
    ) -> bool: ...

weaviate/warnings.py:270

  • stacklevel=3 will surface the warning location at delete_property_index() inside the library rather than at the user’s call site. If the intent is to guide users to update their code, this should usually be one level higher.
            message="""Dep030: Passing a string `index_name` is deprecated and will be removed in a future release.
            Pass an `InvertedIndexType` value instead (e.g. `InvertedIndexType.SEARCHABLE`).""",
            category=DeprecationWarning,
            stacklevel=3,
        )

weaviate/collections/config/sync.pyi:98

  • The PR deprecates passing a raw string index_name to delete_property_index, but the type stub currently models this as a non-deprecated union. Adding deprecated overloads lets Pyright/IDE tooling flag the deprecated call path while keeping it type-correct.
    def delete_property_index(
        self, property_name: str, index_name: Union[InvertedIndexType, IndexName]
    ) -> bool: ...

Comment thread weaviate/classes/config.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support for Runtime Property Reindex

4 participants