mirror of
https://github.com/lancedb/lancedb.git
synced 2026-10-27 14:06:14 +00:00
fix(python): restore sync remote connection and table API parity (#4473)
Fixes #4400. Sync remote connections fail on `uri`, `len(db)`, and `name in db`, and remote tables lack `namespace`, `id`, and `replace_field_metadata`. Share the paginated connection helpers and table identifier contract through the base classes, and delegate remote metadata replacement to the existing async implementation. Unsupported remote session and read-consistency properties raise explicit `NotImplementedError`s. Reconciled with main at `3dd57978`, including the synchronous method signatures from #4498. The local session fix has already landed in #4433, so this PR preserves its supplied-session-or-None contract and adds no Rust/PyO3 session access. Namespace connections retain their writable properties. Existing custom `Table` subclasses remain instantiable: `namespace` defaults to a fresh empty list, and deprecated `replace_field_metadata` delegates to `update_field_metadata` with `replace=True` while retaining its `None` return. Native and remote overrides are preserved. Regression tests instantiate an old-style subclass through normal ABC class creation and verify identifiers, independent namespace lists, deprecation, and exact metadata delegation. Historical validation at `03c43f7b` before the latest main integration and subclass compatibility repair, Ubuntu x86-64, Python 3.12.13: - Rebuilt the native extension with the repository `ci` profile and locked Python dependencies. - 273 tests passed across `test_sync_api_parity.py`, `test_db.py`, `test_remote_db.py`, and `test_session.py`. - 84 namespace and catalog compatibility tests passed (357 tests total). - Python SDK lint and formatting pass (158 files); changed-file checks and `git diff --check` pass. - Repository-wide lint/format checks report existing issues in unchanged `docs/web-tests` files (49 lint findings and 11 formatting files). These are outside this patch. Remote tests use the existing HTTP fixtures through the rebuilt native client; the original aarch64 server deployment was not recreated. Validation after merging main `3dd57978` and the compatibility repair (macOS ARM64, uv-managed Python 3.12.12, rebuilt editable native extension): - 60 focused parity, signature, and error-contract tests passed. - 381 database, remote, session, namespace, catalog, and sync parity tests passed; 2 platform-dependent tests skipped. - The two new subclass regressions fail against the pre-repair implementation with the expected abstract-member instantiation error, then pass with the repair. - Python SDK Ruff lint/format passes (167 files); repository-wide findings remain confined to unchanged docs/web-tests (49 lint findings, 11 formatting files). --------- Co-authored-by: Xuanwo <github@xuanwo.io>
This commit is contained in:
+18
-15
@@ -823,8 +823,26 @@ class DBConnection(EnforceOverrides):
|
||||
|
||||
@property
|
||||
def uri(self) -> str:
|
||||
"""The URI of this database."""
|
||||
return self._uri
|
||||
|
||||
def _all_table_names(self) -> Generator[str, None, None]:
|
||||
page_token = None
|
||||
while True:
|
||||
response = self.list_tables(page_token=page_token)
|
||||
yield from response.tables
|
||||
page_token = response.page_token
|
||||
if not page_token:
|
||||
return
|
||||
|
||||
def __len__(self) -> int:
|
||||
"""Return the number of tables in the root namespace across all pages."""
|
||||
return sum(1 for _ in self._all_table_names())
|
||||
|
||||
def __contains__(self, name: str) -> bool:
|
||||
"""Return whether the root namespace contains the named table."""
|
||||
return name in self._all_table_names()
|
||||
|
||||
def namespace_client(self) -> LanceNamespace:
|
||||
"""Get the equivalent namespace client for this connection.
|
||||
|
||||
@@ -1609,21 +1627,6 @@ class LanceDBConnection(DBConnection):
|
||||
)
|
||||
)
|
||||
|
||||
def _all_table_names(self) -> Generator[str, None, None]:
|
||||
page_token = None
|
||||
while True:
|
||||
response = self.list_tables(page_token=page_token)
|
||||
yield from response.tables
|
||||
page_token = response.page_token
|
||||
if not page_token:
|
||||
return
|
||||
|
||||
def __len__(self) -> int:
|
||||
return sum(1 for _ in self._all_table_names())
|
||||
|
||||
def __contains__(self, name: str) -> bool:
|
||||
return name in self._all_table_names()
|
||||
|
||||
@override
|
||||
def create_table(
|
||||
self,
|
||||
|
||||
@@ -47,7 +47,7 @@ from ..secrets import EnvVarSecret, SecretInfo
|
||||
from ..view import ViewDescription
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from .._lancedb import JobInfo
|
||||
from .._lancedb import JobInfo, Session
|
||||
from ..embeddings import EmbeddingFunctionConfig
|
||||
from lance_namespace import (
|
||||
LanceNamespace,
|
||||
@@ -240,6 +240,23 @@ class RemoteDBConnection(DBConnection):
|
||||
def __repr__(self) -> str:
|
||||
return f"RemoteConnect(name={self.db_name})"
|
||||
|
||||
@property
|
||||
@override
|
||||
def uri(self) -> str:
|
||||
return self._conn.uri
|
||||
|
||||
@property
|
||||
def session(self) -> Optional["Session"]:
|
||||
"""Remote connections do not expose a local cache session."""
|
||||
raise NotImplementedError("session is not supported by remote connections")
|
||||
|
||||
@property
|
||||
def read_consistency_interval(self) -> Optional[timedelta]:
|
||||
"""Remote connections do not expose a local read consistency interval."""
|
||||
raise NotImplementedError(
|
||||
"read_consistency_interval is not supported by remote connections"
|
||||
)
|
||||
|
||||
@override
|
||||
def serialize(self) -> str:
|
||||
if getattr(self, "_catalog_oauth", False):
|
||||
|
||||
@@ -192,6 +192,18 @@ class RemoteTable(Table):
|
||||
"""The name of the table"""
|
||||
return self._name
|
||||
|
||||
@property
|
||||
def namespace(self) -> List[str]:
|
||||
return self._namespace_path
|
||||
|
||||
@deprecation.deprecated(
|
||||
deprecated_in="0.33.1",
|
||||
current_version=__version__,
|
||||
details="Use update_field_metadata() instead.",
|
||||
)
|
||||
def replace_field_metadata(self, field_name: str, new_metadata: Dict[str, str]):
|
||||
LOOP.run(self._table.replace_field_metadata(field_name, new_metadata))
|
||||
|
||||
def __repr__(self) -> str:
|
||||
return f"RemoteTable({self.db_name}.{self.name})"
|
||||
|
||||
|
||||
@@ -1360,6 +1360,16 @@ class Table(ABC):
|
||||
"""The name of this Table"""
|
||||
raise NotImplementedError("name is not supported for this table type")
|
||||
|
||||
@property
|
||||
def namespace(self) -> List[str]:
|
||||
"""Return the namespace path of the table (empty for the root namespace)."""
|
||||
return []
|
||||
|
||||
@property
|
||||
def id(self) -> str:
|
||||
"""Return the full identifier of the table (namespace$name)."""
|
||||
return "$".join(self.namespace + [self.name])
|
||||
|
||||
@property
|
||||
@abstractmethod
|
||||
def version(self) -> int:
|
||||
@@ -3106,6 +3116,29 @@ class Table(ABC):
|
||||
version: the new table version after the update.
|
||||
"""
|
||||
|
||||
@deprecation.deprecated(
|
||||
deprecated_in="0.33.1",
|
||||
current_version=__version__,
|
||||
details="Use update_field_metadata() instead.",
|
||||
)
|
||||
def replace_field_metadata(self, field_name: str, new_metadata: Dict[str, str]):
|
||||
"""Replace all metadata of a field in the schema.
|
||||
|
||||
.. deprecated:: 0.33.1
|
||||
Use [update_field_metadata][lancedb.table.Table.update_field_metadata]
|
||||
with ``replace=True`` instead.
|
||||
|
||||
Parameters
|
||||
----------
|
||||
field_name: str
|
||||
The name of the field whose metadata is replaced.
|
||||
new_metadata: dict
|
||||
The new metadata, replacing all existing keys.
|
||||
"""
|
||||
self.update_field_metadata(
|
||||
{"path": field_name, "metadata": new_metadata, "replace": True}
|
||||
)
|
||||
|
||||
@abstractmethod
|
||||
def drop_columns(self, columns: Iterable[str]) -> DropColumnsResult:
|
||||
"""
|
||||
@@ -3315,13 +3348,6 @@ class LanceTable(Table):
|
||||
"""Return the namespace path of the table."""
|
||||
return self._namespace_path
|
||||
|
||||
@property
|
||||
def id(self) -> str:
|
||||
"""Return the full identifier of the table (namespace$name)."""
|
||||
if self._namespace_path:
|
||||
return "$".join(self._namespace_path + [self.name])
|
||||
return self.name
|
||||
|
||||
@classmethod
|
||||
async def from_inner(cls, tbl: LanceDBTable):
|
||||
from .db import AsyncConnection, LanceDBConnection
|
||||
|
||||
@@ -0,0 +1,170 @@
|
||||
# SPDX-License-Identifier: Apache-2.0
|
||||
# SPDX-FileCopyrightText: Copyright The LanceDB Authors
|
||||
|
||||
from datetime import timedelta
|
||||
import json
|
||||
from urllib.parse import parse_qs, urlparse
|
||||
|
||||
import lancedb
|
||||
from lancedb.table import Table
|
||||
import pytest
|
||||
|
||||
from test_remote_db import mock_lancedb_connection
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def legacy_table():
|
||||
"""Implement the pre-existing abstract API without the two new members."""
|
||||
|
||||
def unsupported(self, *args, **kwargs):
|
||||
raise NotImplementedError
|
||||
|
||||
def update_field_metadata(self, updates):
|
||||
self.metadata_updates.append(updates)
|
||||
return object() # The deprecated wrapper must not expose this result.
|
||||
|
||||
# Supply concrete implementations through normal ABC class creation. Leave
|
||||
# namespace and replace_field_metadata inherited, as an old subclass would.
|
||||
implementations = {
|
||||
name: unsupported
|
||||
for name in Table.__abstractmethods__
|
||||
if name not in {"namespace", "replace_field_metadata"}
|
||||
}
|
||||
implementations.update(
|
||||
name=property(lambda self: "legacy"),
|
||||
update_field_metadata=update_field_metadata,
|
||||
)
|
||||
legacy_class = type("LegacyTable", (Table,), implementations)
|
||||
table = legacy_class()
|
||||
table.metadata_updates = []
|
||||
return table
|
||||
|
||||
|
||||
def test_legacy_table_namespace_and_id(legacy_table):
|
||||
assert legacy_table.namespace == []
|
||||
assert legacy_table.id == "legacy"
|
||||
namespace = legacy_table.namespace
|
||||
namespace.append("changed")
|
||||
assert legacy_table.namespace == []
|
||||
assert legacy_table.id == "legacy"
|
||||
|
||||
|
||||
def test_legacy_table_replace_field_metadata(legacy_table):
|
||||
with pytest.warns(DeprecationWarning, match="update_field_metadata"):
|
||||
result = legacy_table.replace_field_metadata("nested.field", {"a": "b"})
|
||||
assert result is None
|
||||
assert legacy_table.metadata_updates == [
|
||||
{"path": "nested.field", "metadata": {"a": "b"}, "replace": True}
|
||||
]
|
||||
|
||||
|
||||
def respond(request, body):
|
||||
request.send_response(200)
|
||||
request.send_header("Content-Type", "application/json")
|
||||
request.end_headers()
|
||||
request.wfile.write(json.dumps(body).encode())
|
||||
|
||||
|
||||
def test_remote_uri():
|
||||
with mock_lancedb_connection(lambda request: respond(request, {})) as db:
|
||||
assert db.uri == "db://dev"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("attribute", ["session", "read_consistency_interval"])
|
||||
def test_remote_unsupported_properties(attribute):
|
||||
with mock_lancedb_connection(lambda request: respond(request, {})) as db:
|
||||
with pytest.raises(NotImplementedError, match=attribute):
|
||||
getattr(db, attribute)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("tables", [[], ["first", "last"]])
|
||||
def test_remote_len_and_contains_follow_pages(tables):
|
||||
requests = []
|
||||
|
||||
def handler(request):
|
||||
requests.append(request.path)
|
||||
query = parse_qs(urlparse(request.path).query)
|
||||
if tables and "page_token" not in query:
|
||||
respond(request, {"tables": [tables[0]], "page_token": "opaque-next"})
|
||||
else:
|
||||
if tables:
|
||||
assert query["page_token"] == ["opaque-next"]
|
||||
respond(request, {"tables": tables[1:]})
|
||||
|
||||
with mock_lancedb_connection(handler) as db:
|
||||
assert len(db) == len(tables)
|
||||
assert ("last" in db) == bool(tables)
|
||||
assert "missing" not in db
|
||||
requests.clear()
|
||||
assert ("first" in db) == bool(tables)
|
||||
assert len(requests) == 1
|
||||
|
||||
|
||||
@pytest.mark.parametrize("namespace", [[], ["parent", "child"]])
|
||||
def test_remote_table_identifiers(namespace):
|
||||
def handler(request):
|
||||
respond(request, {"version": 1, "schema": {"fields": []}})
|
||||
|
||||
with mock_lancedb_connection(handler) as db:
|
||||
table = db.open_table("rfm", namespace_path=namespace)
|
||||
assert table.namespace == namespace
|
||||
assert table.id == "$".join(namespace + ["rfm"])
|
||||
|
||||
|
||||
def test_remote_replace_field_metadata():
|
||||
updates = []
|
||||
|
||||
def handler(request):
|
||||
if request.path.endswith("/describe/"):
|
||||
respond(request, {"version": 1, "schema": {"fields": []}})
|
||||
else:
|
||||
assert request.path == "/v1/table/rfm/update_field_metadata/"
|
||||
body = request.rfile.read(int(request.headers["Content-Length"]))
|
||||
updates.append(json.loads(body))
|
||||
respond(request, {"version": 2})
|
||||
|
||||
with mock_lancedb_connection(handler) as db:
|
||||
table = db.open_table("rfm")
|
||||
with pytest.warns(DeprecationWarning, match="update_field_metadata"):
|
||||
assert table.replace_field_metadata("id", {"a": "b"}) is None
|
||||
assert updates == [
|
||||
{"updates": [{"path": "id", "metadata": {"a": "b"}, "replace": True}]}
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("custom_session", [False, True])
|
||||
def test_local_properties_and_metadata(tmp_path, custom_session):
|
||||
supplied = lancedb.Session() if custom_session else None
|
||||
db = lancedb.connect(
|
||||
tmp_path, session=supplied, read_consistency_interval=timedelta(seconds=5)
|
||||
)
|
||||
session = db.session
|
||||
assert session is supplied
|
||||
before = session.size_bytes if session is not None else 0
|
||||
table = db.create_table("rfm", [{"id": i} for i in range(3)])
|
||||
assert table.to_arrow().num_rows == 3
|
||||
if supplied is not None:
|
||||
assert supplied.size_bytes > before
|
||||
assert db.read_consistency_interval == timedelta(seconds=5)
|
||||
assert table.namespace == []
|
||||
assert table.id == "rfm"
|
||||
with pytest.warns(DeprecationWarning):
|
||||
table.replace_field_metadata("id", {"old": "value"})
|
||||
with pytest.warns(DeprecationWarning):
|
||||
table.replace_field_metadata("id", {"a": "b"})
|
||||
assert table.schema.field("id").metadata == {b"a": b"b"}
|
||||
|
||||
|
||||
def test_namespace_connection_properties_remain_writable(tmp_path):
|
||||
session = lancedb.Session()
|
||||
interval = timedelta(seconds=5)
|
||||
with lancedb.connect_namespace(
|
||||
"dir",
|
||||
{"root": str(tmp_path)},
|
||||
session=session,
|
||||
read_consistency_interval=interval,
|
||||
) as db:
|
||||
assert db.session is session
|
||||
assert db.read_consistency_interval == interval
|
||||
db.read_consistency_interval = timedelta(0)
|
||||
assert db.read_consistency_interval == timedelta(0)
|
||||
Reference in New Issue
Block a user