mirror of
https://github.com/lancedb/lancedb.git
synced 2026-10-27 05:56:14 +00:00
fix(python): align sync remote methods with async behavior (#4411)
Sync remote table and connection methods inherited bare base-class stubs, so unsupported conversion and bulk-drop operations raised `NotImplementedError` with an empty message and differed from the async API. Delegate `to_lance()`, `drop_all_tables()`, and `drop_database()` to the async implementation. Sync and async remote Lance conversion share #4306's native-table guard and reject before requesting a URI or opening client storage. Conversion documentation directs remote reads through server queries. Add an explicit Polars error, suggest query conversions in Arrow/Pandas/Polars errors, and give the remaining base-class errors an operation name. Preserve upstream `blob_mode` validation for remote Arrow and Pandas conversion. The shared remote bulk-drop error suggests dropping tables individually. Regression tests cover sync/async rejection parity, accessible and missing advertised locations, pinned versions, and bulk-drop rejection without requests. Sync blob query tests verify that descriptions come from the server and unsupported modes cannot open client storage. Keep the remote closed-blob-handle assertion consistent with the upstream `ValueError` contract. Validation: - All 30 focused regression cases passed. - 222 Python tests passed with rebuilt bindings in the locked uv environment: the full remote and error suites, plus selected local conversion, blob, and deletion tests. - Python lint and formatting, Rust formatting, `cargo check --quiet --features remote --tests --examples`, and `cargo clippy --quiet --features remote --tests --examples` passed. - Root Ruff checking reports 49 existing errors in upstream `docs/web-tests`; those files are unchanged relative to `main`. Fixes #4392 <!-- lance-gatekeeper-fix:v1 agent=95db5dd7002043a2c48924ded18b51b2 generation=1 --> --------- Co-authored-by: Gatefixer <313497061+lancedb-gatefixer[bot]@users.noreply.github.com> Co-authored-by: Xuanwo <github@xuanwo.io>
This commit is contained in:
co-authored by
Xuanwo
parent
0dd78656b9
commit
dd4795028b
@@ -857,10 +857,116 @@ def test_table_unimplemented_functions():
|
||||
|
||||
with mock_lancedb_connection(handler) as db:
|
||||
table = db.create_table("test", [{"id": 1}])
|
||||
with pytest.raises(NotImplementedError):
|
||||
table.to_arrow()
|
||||
with pytest.raises(NotImplementedError):
|
||||
table.to_pandas()
|
||||
for method in ["to_arrow", "to_pandas", "to_polars"]:
|
||||
with pytest.raises(NotImplementedError) as exc_info:
|
||||
getattr(table, method)()
|
||||
message = str(exc_info.value)
|
||||
assert method in message
|
||||
assert "not" in message and "supported" in message
|
||||
assert "remote tables" in message
|
||||
assert "cloud" not in message.lower()
|
||||
assert f"search().{method}()" in message
|
||||
|
||||
|
||||
@pytest.mark.parametrize("method", ["to_arrow", "to_pandas"])
|
||||
@pytest.mark.parametrize("blob_mode", ["lazy", "descriptions", "bytes", "invalid"])
|
||||
def test_remote_conversion_blob_mode_errors(method, blob_mode):
|
||||
with blob_remote_table() as table:
|
||||
if blob_mode == "invalid":
|
||||
with pytest.raises(ValueError, match="blob_mode must be one of"):
|
||||
getattr(table, method)(blob_mode=blob_mode)
|
||||
else:
|
||||
with pytest.raises(NotImplementedError) as error:
|
||||
getattr(table, method)(blob_mode=blob_mode)
|
||||
assert f"search().{method}()" in str(error.value)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize("version", [None, 1])
|
||||
@pytest.mark.parametrize("expose_location", [False, True])
|
||||
async def test_remote_to_lance_rejection(
|
||||
tmp_path, version, expose_location, monkeypatch
|
||||
):
|
||||
import lance
|
||||
|
||||
location = tmp_path / "test.lance"
|
||||
lance.write_dataset(pa.table({"id": [1]}), location)
|
||||
lance.write_dataset(pa.table({"id": [2]}), location, mode="append")
|
||||
requests = []
|
||||
|
||||
def fail_dataset(*args, **kwargs):
|
||||
raise AssertionError("remote storage must not be opened")
|
||||
|
||||
monkeypatch.setattr(lance, "dataset", fail_dataset)
|
||||
|
||||
def handler(request):
|
||||
requests.append(request.path)
|
||||
assert request.path == "/v1/table/test/describe/"
|
||||
content_len = int(request.headers.get("Content-Length", 0))
|
||||
body = json.loads(request.rfile.read(content_len)) if content_len else {}
|
||||
request.send_response(200)
|
||||
request.send_header("Content-Type", "application/json")
|
||||
request.end_headers()
|
||||
response = {
|
||||
"version": body.get("version") or 2,
|
||||
"schema": {"fields": []},
|
||||
}
|
||||
if expose_location:
|
||||
response["location"] = str(location)
|
||||
request.wfile.write(json.dumps(response).encode())
|
||||
|
||||
options = {"default_scan_options": {"with_row_id": True}}
|
||||
with mock_lancedb_connection(handler) as db:
|
||||
table = db.open_table("test", version=version)
|
||||
opened_requests = list(requests)
|
||||
with pytest.raises(NotImplementedError) as sync_error:
|
||||
table.to_lance(**options)
|
||||
assert requests == opened_requests
|
||||
|
||||
async with mock_lancedb_connection_async(handler) as db:
|
||||
table = await db.open_table("test", version=version)
|
||||
opened_requests = list(requests)
|
||||
with pytest.raises(NotImplementedError) as async_error:
|
||||
await table.to_lance(**options)
|
||||
assert requests == opened_requests
|
||||
|
||||
assert "to_lance() is not supported for remote tables" in str(sync_error.value)
|
||||
assert "query the server" in str(sync_error.value)
|
||||
assert str(sync_error.value) == str(async_error.value)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
"method, kwargs",
|
||||
[
|
||||
("drop_all_tables", {}),
|
||||
("drop_all_tables", {"namespace_path": []}),
|
||||
("drop_all_tables", {"namespace_path": ["team"]}),
|
||||
("drop_database", {}),
|
||||
],
|
||||
)
|
||||
async def test_remote_drop_all_tables_not_supported(method, kwargs):
|
||||
requests = []
|
||||
|
||||
def handler(request):
|
||||
requests.append(request.path)
|
||||
request.send_response(500)
|
||||
request.end_headers()
|
||||
|
||||
with mock_lancedb_connection(handler) as db:
|
||||
with pytest.raises(NotImplementedError) as sync_error:
|
||||
getattr(db, method)(**kwargs)
|
||||
|
||||
async with mock_lancedb_connection_async(handler) as db:
|
||||
with pytest.raises(NotImplementedError) as async_error:
|
||||
await getattr(db, method)(**kwargs)
|
||||
|
||||
assert "Dropping all tables is not currently supported in the remote API" in str(
|
||||
sync_error.value
|
||||
)
|
||||
assert "drop_table" in str(sync_error.value)
|
||||
assert str(sync_error.value) == str(async_error.value)
|
||||
assert requests == []
|
||||
|
||||
|
||||
def test_table_to_pandas_not_supported():
|
||||
@@ -1681,10 +1787,13 @@ def test_remote_python_only_messages(caplog):
|
||||
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 method in ["to_arrow", "to_pandas"]:
|
||||
for method in ["to_arrow", "to_pandas", "to_polars"]:
|
||||
with pytest.raises(NotImplementedError) as error:
|
||||
getattr(table, method)()
|
||||
assert str(error.value) == f"{method}() is not supported for remote tables."
|
||||
assert str(error.value) == (
|
||||
f"{method}() is not supported for remote tables. "
|
||||
f"Use table.search().{method}() instead."
|
||||
)
|
||||
for method in ["compact_files", "cleanup_old_versions"]:
|
||||
with pytest.warns(UserWarning) as warning:
|
||||
getattr(table, method)()
|
||||
@@ -3490,14 +3599,17 @@ def blob_descriptor_take_table():
|
||||
|
||||
|
||||
@contextlib.contextmanager
|
||||
def blob_remote_table(*, server_version=Version("0.5.0")):
|
||||
def blob_remote_table(*, server_version=Version("0.5.0"), location=None):
|
||||
def handler(request):
|
||||
if request.path == "/v1/table/test/describe/":
|
||||
request.send_response(200)
|
||||
request.send_header("Content-Type", "application/json")
|
||||
request.send_header("phalanx-version", str(server_version))
|
||||
request.end_headers()
|
||||
request.wfile.write(json.dumps(BLOB_DESCRIBE_RESPONSE).encode())
|
||||
response = dict(BLOB_DESCRIBE_RESPONSE)
|
||||
if location is not None:
|
||||
response["location"] = str(location)
|
||||
request.wfile.write(json.dumps(response).encode())
|
||||
elif request.path.startswith("/v1/table/test/blob/image/"):
|
||||
path = request.path.partition("?")[0]
|
||||
row_id = int(path.split("/")[-2])
|
||||
@@ -3532,7 +3644,6 @@ def blob_remote_table(*, server_version=Version("0.5.0")):
|
||||
if columns == ["id"]:
|
||||
response_table = blob_query_response_table().select(["id"])
|
||||
else:
|
||||
assert body["with_row_id"] is True
|
||||
if columns == ["image"]:
|
||||
# Blob handles are sized from a descriptor take.
|
||||
assert body["filter"].startswith("_rowid IN")
|
||||
@@ -3545,6 +3656,8 @@ def blob_remote_table(*, server_version=Version("0.5.0")):
|
||||
)
|
||||
else:
|
||||
response_table = blob_query_response_table()
|
||||
if not body.get("with_row_id", False):
|
||||
response_table = response_table.drop(["_rowid"])
|
||||
request.send_response(200)
|
||||
request.send_header("Content-Type", "application/vnd.apache.arrow.file")
|
||||
request.send_header("x-lancedb-version", "1")
|
||||
@@ -3579,6 +3692,32 @@ def test_remote_blob_columns_and_fetch():
|
||||
assert blobs.to_pylist() == [b"alpha", None, b"gamma"]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("blob_mode", ["descriptions", "bytes", "lazy"])
|
||||
def test_sync_remote_blob_pandas_avoids_storage_access(
|
||||
tmp_path, monkeypatch, blob_mode
|
||||
):
|
||||
import lance
|
||||
|
||||
location = tmp_path / "remote.lance"
|
||||
lance.write_dataset(pa.table({"id": [7], "image": [b"server-owned"]}), location)
|
||||
|
||||
def fail_dataset(*args, **kwargs):
|
||||
raise AssertionError("remote storage must not be opened")
|
||||
|
||||
monkeypatch.setattr(lance, "dataset", fail_dataset)
|
||||
with blob_remote_table(location=location) as table:
|
||||
monkeypatch.setattr(table, "to_lance", fail_dataset)
|
||||
query = table.search().select(["id", "image"])
|
||||
if blob_mode == "descriptions":
|
||||
df = query.to_pandas(blob_mode=blob_mode)
|
||||
assert df["id"].tolist() == [1, 2, 3]
|
||||
assert "_rowid" not in df.columns
|
||||
assert df["image"][0]["size"] == 5
|
||||
else:
|
||||
with pytest.raises(NotImplementedError, match="stable table snapshot"):
|
||||
query.to_pandas(blob_mode=blob_mode)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize("use_async", [False, True])
|
||||
async def test_remote_fetch_blob_ranges(use_async):
|
||||
|
||||
Reference in New Issue
Block a user