feat: deprecate async_enabled replication argument removed server-side - #2050
feat: deprecate async_enabled replication argument removed server-side#2050LijuanTang94 wants to merge 1 commit into
async_enabled replication argument removed server-side#2050Conversation
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 |
|
To avoid any confusion in the future about your contribution to Weaviate, we work with a Contributor License Agreement. If you agree, you can simply add a comment to this PR that you agree with the CLA so that we can merge. |
|
I have read the CLA and agree to the terms. |
|
Hi team — just checking in on this PR. CLA is now signed and all checks are passing. Happy to make any changes if needed! |
The `asyncEnabled` field was removed from the Weaviate server schema in v1.38. The client still accepted `async_enabled` and forwarded it, so the server silently dropped it and the user's intent was lost without any indication (issue weaviate#2047). Following the existing precedent for `max_workers` / `alive_nodes_checking_frequency` (Dep029), the `async_enabled` argument in `Configure.replication` / `Reconfigure.replication` now emits a `DeprecationWarning` (Dep030) when passed, while remaining functional against older servers. Docstrings are updated to document the removal. Closes weaviate#2047
c6a1388 to
dce402a
Compare
|
Rebased onto current Heads up on why this PR shows no test signal: the In the meantime I ran the unit suite locally against current On the change itself — this is still live on server v1.38+: One decision I'd like a steer on, which I left out deliberately to keep this non-breaking: the read path ( @jeroiraz — you filed #2047, so you're probably the right person for the read-path question. |
Summary
Closes #2047
The
asyncEnabledfield was removed from the Weaviate server schema in v1.38 (weaviate/weaviate#11214). The client still acceptedasync_enabledand forwarded it on the write path, so the server silently dropped it — the user's intent was lost without any indication.This PR adds a
DeprecationWarningso the silent loss becomes visible, following the exact precedent the maintainers established formax_workers/alive_nodes_checking_frequency(Dep029) when thoseasync_configfields were removed in v1.37.3.Approach
I deliberately chose deprecate-and-warn over hard removal (the issue's step 3 offers both):
async_enabledkwarg outright would break any existing user code callingConfigure.replication(async_enabled=...).If the maintainers prefer to also drop the read-path field (
_ReplicationConfig.async_enabled+ the parser inconfig_methods.py, which now always reports a misleadingFalse), I'm happy to extend the PR — I left it out to keep this change focused and non-breaking, and because removing a read attribute is a larger compatibility decision.Changes
weaviate/warnings.py— newDep030deprecation warningweaviate/collections/classes/config.py—Configure.replicationandReconfigure.replicationemit the warning whenasync_enabledis passed; docstrings updatedtest/collection/test_config.py— tests that the warning fires whenasync_enabledis passed and stays silent otherwise (both factories)Testing