mirror of
https://github.com/lancedb/lancedb.git
synced 2026-11-13 14:16:54 +00:00
fix(python): preserve request metadata for missing remote tables
Fixes #4750 Remote missing-table errors retained HTTP metadata in Rust but the Python binding converted them to a plain `ValueError`, discarding the request ID and status. This raises a `TableNotFoundError` that preserves the original message, request ID, status code, and chained HTTP cause while remaining compatible with existing `ValueError` and `lance_namespace.errors.TableNotFoundError` handlers. Local and non-HTTP errors retain their previous behavior. Validation covers sync and async opens across cached/uncached and root/nested paths, exception pickling, local compatibility, the remote and error test suites, Clippy, formatting, Ruff, and all applicable PR checks including CodeQL.
This commit is contained in:
@@ -4,6 +4,8 @@
|
||||
|
||||
from typing import Optional
|
||||
|
||||
from lance_namespace.errors import TableNotFoundError as _NamespaceTableNotFoundError
|
||||
|
||||
|
||||
class LanceDBClientError(RuntimeError):
|
||||
"""An error that occurred in the LanceDB client.
|
||||
@@ -49,6 +51,26 @@ class HttpError(LanceDBClientError):
|
||||
pass
|
||||
|
||||
|
||||
class TableNotFoundError(HttpError, ValueError, _NamespaceTableNotFoundError):
|
||||
"""A remote table was not found.
|
||||
|
||||
This is also a ``lance_namespace.errors.TableNotFoundError`` and a
|
||||
``ValueError``, so existing missing-table handlers continue to work.
|
||||
Like [HttpError][lancedb.remote.errors.HttpError], it carries the
|
||||
``request_id`` and ``status_code`` of the failed request.
|
||||
|
||||
Examples
|
||||
--------
|
||||
>>> error = TableNotFoundError("Table 'missing' was not found", "req-123", 404)
|
||||
>>> isinstance(error, ValueError)
|
||||
True
|
||||
>>> error.request_id
|
||||
'req-123'
|
||||
"""
|
||||
|
||||
pass
|
||||
|
||||
|
||||
class RetryError(LanceDBClientError):
|
||||
"""An error that occurs when the client has exceeded the maximum number of retries.
|
||||
|
||||
|
||||
@@ -4,9 +4,16 @@
|
||||
import pickle
|
||||
|
||||
import pytest
|
||||
from lance_namespace.errors import ErrorCode
|
||||
from lance_namespace.errors import TableNotFoundError as NamespaceTableNotFoundError
|
||||
|
||||
from lancedb.db import DBConnection
|
||||
from lancedb.remote.errors import HttpError, LanceDBClientError, RetryError
|
||||
from lancedb.remote.errors import (
|
||||
HttpError,
|
||||
LanceDBClientError,
|
||||
RetryError,
|
||||
TableNotFoundError,
|
||||
)
|
||||
from lancedb.table import Table
|
||||
|
||||
|
||||
@@ -52,6 +59,19 @@ def test_pickle_http_error():
|
||||
assert restored.status_code == 404
|
||||
|
||||
|
||||
def test_pickle_table_not_found_error():
|
||||
err = TableNotFoundError("Table 'missing' was not found", "req-missing", 404)
|
||||
restored = pickle.loads(pickle.dumps(err))
|
||||
assert isinstance(restored, TableNotFoundError)
|
||||
assert isinstance(restored, NamespaceTableNotFoundError)
|
||||
assert isinstance(restored, ValueError)
|
||||
assert isinstance(restored, HttpError)
|
||||
assert restored.code == ErrorCode.TABLE_NOT_FOUND
|
||||
assert str(restored) == "Table 'missing' was not found"
|
||||
assert restored.request_id == "req-missing"
|
||||
assert restored.status_code == 404
|
||||
|
||||
|
||||
def test_pickle_retry_error():
|
||||
err = RetryError(
|
||||
"max retries exceeded",
|
||||
|
||||
@@ -21,6 +21,7 @@ from urllib.parse import parse_qs, urlparse
|
||||
from unittest.mock import MagicMock, patch
|
||||
import uuid
|
||||
from packaging.version import Version
|
||||
from lance_namespace.errors import TableNotFoundError
|
||||
|
||||
import lancedb
|
||||
import numpy as np
|
||||
@@ -123,6 +124,72 @@ async def mock_lancedb_connection_async(handler, **client_config):
|
||||
handle.join()
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def missing_table_handler(cached, namespace_path):
|
||||
request_ids = []
|
||||
|
||||
def handler(request):
|
||||
identifier = "$".join(namespace_path + ["missing"])
|
||||
assert request.path == f"/v1/table/{identifier}/describe/"
|
||||
request_ids.append(request.headers["x-request-id"])
|
||||
status = 200 if cached and len(request_ids) == 1 else 404
|
||||
request.send_response(status)
|
||||
request.send_header("Content-Type", "application/json")
|
||||
request.end_headers()
|
||||
request.wfile.write(b"{}" if status == 200 else b"table does not exist")
|
||||
|
||||
return handler, request_ids
|
||||
|
||||
|
||||
@pytest.mark.parametrize("cached", [False, True])
|
||||
@pytest.mark.parametrize("namespace_path", [[], ["ns"]])
|
||||
def test_remote_open_missing_table(cached, namespace_path, missing_table_handler):
|
||||
handler, request_ids = missing_table_handler
|
||||
with mock_lancedb_connection(handler) as db:
|
||||
if cached:
|
||||
db.open_table("missing", namespace_path=namespace_path)
|
||||
with pytest.raises(TableNotFoundError) as exc_info:
|
||||
db.open_table("missing", namespace_path=namespace_path)
|
||||
|
||||
error = exc_info.value
|
||||
assert isinstance(error, ValueError)
|
||||
assert isinstance(error, HttpError)
|
||||
assert str(error) == "Table 'missing' was not found"
|
||||
assert error.status_code == 404
|
||||
assert error.request_id == request_ids[-1]
|
||||
assert error.request_id
|
||||
assert isinstance(error.__cause__, HttpError)
|
||||
assert error.__cause__.request_id == error.request_id
|
||||
assert error.__cause__.status_code == 404
|
||||
assert "table does not exist" in str(error.__cause__)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize("cached", [False, True])
|
||||
@pytest.mark.parametrize("namespace_path", [[], ["ns"]])
|
||||
async def test_async_remote_open_missing_table(
|
||||
cached, namespace_path, missing_table_handler
|
||||
):
|
||||
handler, request_ids = missing_table_handler
|
||||
async with mock_lancedb_connection_async(handler) as db:
|
||||
if cached:
|
||||
await db.open_table("missing", namespace_path=namespace_path)
|
||||
with pytest.raises(TableNotFoundError) as exc_info:
|
||||
await db.open_table("missing", namespace_path=namespace_path)
|
||||
|
||||
error = exc_info.value
|
||||
assert isinstance(error, ValueError)
|
||||
assert isinstance(error, HttpError)
|
||||
assert str(error) == "Table 'missing' was not found"
|
||||
assert error.status_code == 404
|
||||
assert error.request_id == request_ids[-1]
|
||||
assert error.request_id
|
||||
assert isinstance(error.__cause__, HttpError)
|
||||
assert error.__cause__.request_id == error.request_id
|
||||
assert error.__cause__.status_code == 404
|
||||
assert "table does not exist" in str(error.__cause__)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def existence_handler(method, identifier, status):
|
||||
def handler(request):
|
||||
|
||||
+21
-1
@@ -28,12 +28,32 @@ impl<T> PythonErrorExt<T> for std::result::Result<T, LanceError> {
|
||||
Err(err) => match err {
|
||||
LanceError::InvalidInput { .. }
|
||||
| LanceError::InvalidTableName { .. }
|
||||
| LanceError::TableNotFound { .. }
|
||||
| LanceError::NotAMaterializedView { .. }
|
||||
| LanceError::Schema { .. }
|
||||
| LanceError::DatabaseNotFound { .. }
|
||||
| LanceError::DatabaseAlreadyExists { .. }
|
||||
| LanceError::TableAlreadyExists { .. } => self.value_error(),
|
||||
LanceError::TableNotFound { source, .. } => {
|
||||
let Some(LanceError::Http {
|
||||
request_id,
|
||||
status_code,
|
||||
..
|
||||
}) = source.downcast_ref::<LanceError>()
|
||||
else {
|
||||
return self.value_error();
|
||||
};
|
||||
Python::attach(|py| {
|
||||
let cls = py
|
||||
.import(intern!(py, "lancedb.remote.errors"))?
|
||||
.getattr(intern!(py, "TableNotFoundError"))?;
|
||||
let status_code = status_code.map(|s| s.as_u16());
|
||||
let py_err = cls.call1((err.to_string(), request_id, status_code))?;
|
||||
let cause =
|
||||
http_from_rust_error(py, source.as_ref(), request_id, status_code)?;
|
||||
py_err.setattr(intern!(py, "__cause__"), cause)?;
|
||||
Err(PyErr::from_value(py_err))
|
||||
})
|
||||
}
|
||||
LanceError::CreateDir { .. } => self.os_error(),
|
||||
LanceError::ObjectStore { .. } => Err(PyIOError::new_err(err.to_string())),
|
||||
LanceError::NotSupported { .. } => {
|
||||
|
||||
Reference in New Issue
Block a user