mirror of
https://github.com/lancedb/lancedb.git
synced 2026-10-28 06:26:15 +00:00
fix!: reject unsupported remote connection and table options (#4486)
Fixes #4399. Remote connections currently discard local-only settings, including table write options that determine storage format. The synchronous Python client also binds the fourth positional `create_table` argument to `on_bad_vectors`, so an intended overwrite is sent as a create. This change aligns the remote Python signature with `DBConnection.create_table`, preserves the legacy remote timeout keywords, and raises `ValueError` for other unknown connection keywords. The shared Rust client rejects unsupported connection, create, and open options before sending HTTP requests; open validation also runs before a table-cache hit. Python reports these as `NotImplementedError`, including explicit `enable_v2_manifest_paths=False`. Supported Enterprise connection-level storage options remain available. This implements option 2 from the issue: explicit rejection. It deliberately does not add unverified server write parameters. Calls that previously supplied ignored options must omit them to use server-managed storage and caching. Database URI prefixes are already preserved on current main and are unchanged here. Regression tests cover Python sync/async, TypeScript create/open calls, and the Rust remote client, including empty-table creation, overwrite mode, and cached table handles. Python API documentation describes the behavior and corrected positional order. Reconciled with main at `0ea932da`, preserving upstream blob and error-handling tests and updating the Python test that previously expected open options to be ignored. Validation on the merged source: - Rebuilt the Python native extension; 304 tests passed across remote options, database, remote database, and session suites. - Rust remote database suite: 135 passed. Rebuilt the Node native extension; TypeScript remote suite: 62 passed. TypeScript compilation and documentation generation passed. - Full Python SDK lint and formatting pass (159 files); full TypeScript lint and formatting pass (66 files). Rust formatting and `git diff --check` pass. - Repository-wide Python lint/format reports existing issues in unchanged `docs/web-tests` files (49 lint findings and 11 formatting files). Environment: Ubuntu under WSL2, Linux x86-64, CPython 3.12.13, Rust 1.97.0, and Node.js 22.16.0. Locked dependencies and the repository `ci` build profile. Remote tests use HTTP fixtures; this does not claim validation against the original ARM64 single-node deployment. Documentation review follow-up: added a draft Enterprise migration note in `docs/web/basics/sdk.mdx`. It explains that `db://` connections reject local session/cache and per-table storage/write options, while supported connection-level Enterprise storage options remain available. Following the repository's release conventions, the note stays in an MDX comment below the migration list until the behavior is released. Vale 3.24.0 with the pinned LanceDB style package reports no findings; `git diff --check` passes. This follow-up changes documentation only. BREAKING CHANGE: Remote connections now reject unsupported local session/cache and per-table storage/write options instead of silently ignoring them. Callers must omit these options and use server-managed storage and caching; supported Enterprise connection-level storage options remain available. Integration with current main preserves its synchronous API signatures, including the remote `create_table(mode=None)` default, and the table compatibility defaults. The positional-order repair is already on main via #4608; this PR retains the distinct unsupported-option rejection behavior. CI follow-up: the inherited IVF search-effort test occasionally measured recall 0.375 against its unchanged 0.5 threshold after random index training. A matching local reproducer observed this once in 30 builds. The fixture now places the eight exact neighbors together, preserving the 16-partition setup, two writes, all probe/precedence assertions, and the recall threshold. The adjusted fixture returned recall 1.0 in 100 builds; the full query and remote suites passed (102 tests). This stabilizes test data and does not change search behavior or promise production recall. --------- Co-authored-by: Xuanwo <github@xuanwo.io>
This commit is contained in:
@@ -281,6 +281,21 @@ def connect(
|
||||
"uri is required when not connecting through a namespace client"
|
||||
)
|
||||
if isinstance(uri, str) and uri.startswith("db://"):
|
||||
# Preserve the deprecated remote timeout options, but validate all other
|
||||
# unknown keywords just as the local connection does.
|
||||
unknown_kwargs = {
|
||||
key: value
|
||||
for key, value in kwargs.items()
|
||||
if key not in ("connection_timeout", "read_timeout")
|
||||
}
|
||||
if unknown_kwargs:
|
||||
raise ValueError(f"Unknown keyword arguments: {unknown_kwargs}")
|
||||
if session is not None:
|
||||
raise NotImplementedError("session is not supported for remote connections")
|
||||
if manifest_enabled:
|
||||
raise NotImplementedError(
|
||||
"manifest_enabled is not supported for remote connections"
|
||||
)
|
||||
if api_key is None:
|
||||
api_key = os.environ.get("LANCEDB_API_KEY")
|
||||
if api_key is None:
|
||||
|
||||
@@ -7,7 +7,6 @@ from __future__ import annotations
|
||||
from dataclasses import replace
|
||||
from datetime import timedelta
|
||||
import json
|
||||
import logging
|
||||
from concurrent.futures import ThreadPoolExecutor
|
||||
import sys
|
||||
from typing import (
|
||||
@@ -502,18 +501,14 @@ class RemoteDBConnection(DBConnection):
|
||||
|
||||
if namespace_path is None:
|
||||
namespace_path = []
|
||||
if storage_options is not None:
|
||||
logging.info(
|
||||
"storage_options is ignored for remote tables"
|
||||
" (storage is managed; set storage_options on connect() instead)"
|
||||
table = LOOP.run(
|
||||
self._conn.open_table(
|
||||
name,
|
||||
namespace_path=namespace_path,
|
||||
storage_options=storage_options,
|
||||
index_cache_size=index_cache_size,
|
||||
)
|
||||
if index_cache_size is not None:
|
||||
logging.info(
|
||||
"index_cache_size is ignored for remote tables"
|
||||
" (there is no local cache to configure)"
|
||||
)
|
||||
|
||||
table = LOOP.run(self._conn.open_table(name, namespace_path=namespace_path))
|
||||
)
|
||||
tbl = RemoteTable(
|
||||
table,
|
||||
self.db_name,
|
||||
@@ -593,6 +588,9 @@ class RemoteDBConnection(DBConnection):
|
||||
embedding_functions: Optional[List[EmbeddingFunctionConfig]] = None,
|
||||
*,
|
||||
namespace_path: Optional[List[str]] = None,
|
||||
storage_options: Optional[Dict[str, str]] = None,
|
||||
data_storage_version: Optional[str] = None,
|
||||
enable_v2_manifest_paths: Optional[bool] = None,
|
||||
) -> Table:
|
||||
"""Create a [Table][lancedb.table.Table] in the database.
|
||||
|
||||
@@ -632,6 +630,12 @@ class RemoteDBConnection(DBConnection):
|
||||
embedding_functions: list of EmbeddingFunctionConfig, optional
|
||||
The embedding functions to store in the table schema and use to
|
||||
generate vectors when creating the table or adding data.
|
||||
storage_options: dict, optional
|
||||
Not supported for remote table creation. Raises NotImplementedError.
|
||||
data_storage_version: str, optional
|
||||
Not supported for remote table creation. Raises NotImplementedError.
|
||||
enable_v2_manifest_paths: bool, optional
|
||||
Not supported for remote table creation. Raises NotImplementedError.
|
||||
|
||||
Returns
|
||||
-------
|
||||
@@ -698,6 +702,14 @@ class RemoteDBConnection(DBConnection):
|
||||
LanceTable(table4)
|
||||
|
||||
"""
|
||||
for option, value in (
|
||||
("data_storage_version", data_storage_version),
|
||||
("enable_v2_manifest_paths", enable_v2_manifest_paths),
|
||||
):
|
||||
if value is not None:
|
||||
raise NotImplementedError(
|
||||
f"{option} is not supported for remote table creation"
|
||||
)
|
||||
if exist_ok:
|
||||
if mode == "create":
|
||||
mode = "exist_ok"
|
||||
@@ -719,6 +731,7 @@ class RemoteDBConnection(DBConnection):
|
||||
on_bad_vectors=on_bad_vectors,
|
||||
fill_value=fill_value,
|
||||
embedding_functions=embedding_functions,
|
||||
storage_options=storage_options,
|
||||
)
|
||||
)
|
||||
return RemoteTable(
|
||||
|
||||
@@ -1823,12 +1823,12 @@ def test_remote_unsupported_legacy_hnsw_pq():
|
||||
)
|
||||
|
||||
|
||||
def test_remote_python_only_messages(caplog):
|
||||
def test_remote_python_only_messages():
|
||||
with mock_lancedb_connection(remote_unsupported_handler) as db:
|
||||
with caplog.at_level("INFO"):
|
||||
table = db.open_table("test", storage_options={}, index_cache_size=1)
|
||||
assert "storage_options is ignored for remote tables" in caplog.text
|
||||
assert "index_cache_size is ignored for remote tables" in caplog.text
|
||||
for options in [{"storage_options": {}}, {"index_cache_size": 1}]:
|
||||
with pytest.raises(NotImplementedError, match=next(iter(options))):
|
||||
db.open_table("test", **options)
|
||||
table = db.open_table("test")
|
||||
for method in ["to_arrow", "to_pandas", "to_polars"]:
|
||||
with pytest.raises(NotImplementedError) as error:
|
||||
getattr(table, method)()
|
||||
|
||||
@@ -0,0 +1,154 @@
|
||||
# SPDX-License-Identifier: Apache-2.0
|
||||
# SPDX-FileCopyrightText: Copyright The LanceDB Authors
|
||||
|
||||
import inspect
|
||||
from urllib.parse import parse_qs, urlparse
|
||||
|
||||
import lancedb
|
||||
import pyarrow as pa
|
||||
import pytest
|
||||
|
||||
from test_remote_db import mock_lancedb_connection, mock_lancedb_connection_async
|
||||
|
||||
|
||||
@pytest.mark.parametrize("uri", ["memory://", "db://dev"])
|
||||
def test_unknown_connect_options(uri):
|
||||
with pytest.raises(ValueError, match="Unknown keyword arguments.*bogus"):
|
||||
lancedb.connect(uri, api_key="fake", bogus=1)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("option", ["session", "manifest_enabled"])
|
||||
@pytest.mark.parametrize("asynchronous", [False, True])
|
||||
@pytest.mark.asyncio
|
||||
async def test_remote_connect_rejects_local_options(option, asynchronous):
|
||||
value = lancedb.Session() if option == "session" else True
|
||||
with pytest.raises(NotImplementedError, match=option):
|
||||
if asynchronous:
|
||||
await lancedb.connect_async("db://dev", api_key="fake", **{option: value})
|
||||
else:
|
||||
lancedb.connect("db://dev", api_key="fake", **{option: value})
|
||||
|
||||
|
||||
def test_remote_create_signature():
|
||||
from lancedb.db import DBConnection
|
||||
from lancedb.remote.db import RemoteDBConnection
|
||||
|
||||
expected = inspect.signature(DBConnection.create_table).parameters
|
||||
actual = inspect.signature(RemoteDBConnection.create_table).parameters
|
||||
assert list(actual) == list(expected)
|
||||
for name in expected:
|
||||
assert actual[name].kind == expected[name].kind
|
||||
if name == "mode":
|
||||
# Remote connections retain their existing server-default mode.
|
||||
assert actual[name].default is None
|
||||
else:
|
||||
assert actual[name].default == expected[name].default
|
||||
|
||||
|
||||
def test_remote_create_positional_mode():
|
||||
modes = []
|
||||
|
||||
def handler(request):
|
||||
assert urlparse(request.path).path == "/v1/table/ct_pos/create/"
|
||||
modes.append(parse_qs(urlparse(request.path).query)["mode"])
|
||||
request.send_response(200)
|
||||
request.end_headers()
|
||||
|
||||
with mock_lancedb_connection(handler) as db:
|
||||
data = [{"value": 1}]
|
||||
db.create_table("ct_pos", data, None, "overwrite")
|
||||
db.create_table("ct_pos", data, None, "overwrite")
|
||||
db.create_table("ct_pos", data, None, "create", True)
|
||||
with pytest.raises(ValueError, match="Invalid mode"):
|
||||
db.create_table("ct_pos", data, None, "drop")
|
||||
assert modes == [["overwrite"], ["overwrite"], ["exist_ok"]]
|
||||
|
||||
|
||||
CREATE_OPTIONS = [
|
||||
{"new_table_data_storage_version": "2.0"},
|
||||
{"new_table_enable_v2_manifest_paths": "false"},
|
||||
{"bogus_option_xyz": "1"},
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("storage_options", CREATE_OPTIONS)
|
||||
@pytest.mark.parametrize("empty", [False, True])
|
||||
@pytest.mark.asyncio
|
||||
async def test_remote_create_rejects_write_options(storage_options, empty):
|
||||
requests = []
|
||||
|
||||
def handler(request):
|
||||
requests.append(request.path)
|
||||
request.send_response(200)
|
||||
request.end_headers()
|
||||
|
||||
kwargs = {"schema": pa.schema([("value", pa.int64())])}
|
||||
if not empty:
|
||||
kwargs["data"] = [{"value": 1}]
|
||||
async with mock_lancedb_connection_async(handler) as db:
|
||||
with pytest.raises(NotImplementedError, match="write_options.*remote"):
|
||||
await db.create_table("t", storage_options=storage_options, **kwargs)
|
||||
assert requests == []
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"options",
|
||||
[
|
||||
{"data_storage_version": "2.0"},
|
||||
{"enable_v2_manifest_paths": False},
|
||||
{"storage_options": {"x": "1"}},
|
||||
],
|
||||
)
|
||||
def test_sync_remote_create_rejects_write_options(options):
|
||||
requests = []
|
||||
|
||||
def handler(request):
|
||||
requests.append(request.path)
|
||||
request.send_response(200)
|
||||
request.end_headers()
|
||||
|
||||
with mock_lancedb_connection(handler) as db:
|
||||
with pytest.raises(NotImplementedError, match="remote"):
|
||||
db.create_table("t", [{"value": 1}], **options)
|
||||
assert requests == []
|
||||
|
||||
|
||||
@pytest.mark.parametrize("asynchronous", [False, True])
|
||||
@pytest.mark.parametrize(
|
||||
"options", [{"index_cache_size": 10}, {"storage_options": {"x": "y"}}]
|
||||
)
|
||||
@pytest.mark.asyncio
|
||||
async def test_remote_open_rejects_local_options(options, asynchronous):
|
||||
requests = []
|
||||
|
||||
def handler(request):
|
||||
requests.append(request.path)
|
||||
request.send_response(200)
|
||||
request.end_headers()
|
||||
request.wfile.write(b'{"version": 1, "schema": {"fields": []}}')
|
||||
|
||||
if asynchronous:
|
||||
async with mock_lancedb_connection_async(handler) as db:
|
||||
with pytest.raises(NotImplementedError, match="remote"):
|
||||
await db.open_table("t", **options)
|
||||
else:
|
||||
with mock_lancedb_connection(handler) as db:
|
||||
with pytest.raises(NotImplementedError, match="remote"):
|
||||
db.open_table("t", **options)
|
||||
assert requests == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_remote_open_rejects_location():
|
||||
requests = []
|
||||
|
||||
def handler(request):
|
||||
requests.append(request.path)
|
||||
request.send_response(200)
|
||||
request.end_headers()
|
||||
request.wfile.write(b'{"version": 1, "schema": {"fields": []}}')
|
||||
|
||||
async with mock_lancedb_connection_async(handler) as db:
|
||||
with pytest.raises(NotImplementedError, match="location.*remote"):
|
||||
await db.open_table("t", location="/tmp/zzz")
|
||||
assert requests == []
|
||||
Reference in New Issue
Block a user