From f8935893565688be9b6c3b7ee5e2937c44d96f7a Mon Sep 17 00:00:00 2001 From: Brendan Clement Date: Thu, 14 May 2026 15:17:44 -0700 Subject: [PATCH] fix(python): invalid namespace mode/behavior was silently ignored, now raises ValueError (#3388) Follow-up to #3371 , which added runtime validation for namespace `mode` and `behavior` parameters in the NodeJS SDK. Bringing the same fix to Python for cross-SDK consistency. **Before:** unrecognized values were silently dropped to `None`, so `db.create_namespace(["x"], mode="foobar")` would quietly fall through to the server's default mode and hide caller typos. **After:** raises `ValueError` listing the valid values. --- python/python/lancedb/namespace_utils.py | 28 ++++++++++++-- python/python/tests/test_db.py | 23 ++++++++++++ python/src/connection.rs | 47 ++++++++++++++++-------- 3 files changed, 79 insertions(+), 19 deletions(-) diff --git a/python/python/lancedb/namespace_utils.py b/python/python/lancedb/namespace_utils.py index eeeac5142..5b8d3b972 100644 --- a/python/python/lancedb/namespace_utils.py +++ b/python/python/lancedb/namespace_utils.py @@ -6,22 +6,44 @@ from typing import Optional +_CREATE_NAMESPACE_MODES = frozenset({"create", "exist_ok", "overwrite"}) +_DROP_NAMESPACE_MODES = frozenset({"SKIP", "FAIL"}) +_DROP_NAMESPACE_BEHAVIORS = frozenset({"RESTRICT", "CASCADE"}) + + def _normalize_create_namespace_mode(mode: Optional[str]) -> Optional[str]: """Normalize create namespace mode to lowercase (API expects lowercase).""" if mode is None: return None - return mode.lower() + normalized = mode.lower() + if normalized not in _CREATE_NAMESPACE_MODES: + raise ValueError( + f"Invalid create namespace mode {mode!r}: " + f"expected one of 'create', 'exist_ok', 'overwrite'" + ) + return normalized def _normalize_drop_namespace_mode(mode: Optional[str]) -> Optional[str]: """Normalize drop namespace mode to uppercase (API expects uppercase).""" if mode is None: return None - return mode.upper() + normalized = mode.upper() + if normalized not in _DROP_NAMESPACE_MODES: + raise ValueError( + f"Invalid drop namespace mode {mode!r}: expected one of 'skip', 'fail'" + ) + return normalized def _normalize_drop_namespace_behavior(behavior: Optional[str]) -> Optional[str]: """Normalize drop namespace behavior to uppercase (API expects uppercase).""" if behavior is None: return None - return behavior.upper() + normalized = behavior.upper() + if normalized not in _DROP_NAMESPACE_BEHAVIORS: + raise ValueError( + f"Invalid drop namespace behavior {behavior!r}: " + f"expected one of 'restrict', 'cascade'" + ) + return normalized diff --git a/python/python/tests/test_db.py b/python/python/tests/test_db.py index af03e77cd..f1173e2ff 100644 --- a/python/python/tests/test_db.py +++ b/python/python/tests/test_db.py @@ -914,6 +914,29 @@ def test_local_namespace_operations(tmp_path): assert db.list_namespaces().namespaces == [] +def test_create_namespace_invalid_mode_raises(tmp_path): + """Unrecognized create namespace modes raise a clear error.""" + db = lancedb.connect(tmp_path) + with pytest.raises(ValueError, match="Invalid create namespace mode"): + db.create_namespace(["child"], mode="frobnicate") + + +def test_drop_namespace_invalid_mode_raises(tmp_path): + """Unrecognized drop namespace modes raise a clear error.""" + db = lancedb.connect(tmp_path) + db.create_namespace(["child"]) + with pytest.raises(ValueError, match="Invalid drop namespace mode"): + db.drop_namespace(["child"], mode="frobnicate") + + +def test_drop_namespace_invalid_behavior_raises(tmp_path): + """Unrecognized drop namespace behaviors raise a clear error.""" + db = lancedb.connect(tmp_path) + db.create_namespace(["child"]) + with pytest.raises(ValueError, match="Invalid drop namespace behavior"): + db.drop_namespace(["child"], behavior="frobnicate") + + def test_clone_table_latest_version(tmp_path): """Test cloning a table with the latest version (default behavior)""" import os diff --git a/python/src/connection.rs b/python/src/connection.rs index 703b44424..51713fc93 100644 --- a/python/src/connection.rs +++ b/python/src/connection.rs @@ -395,12 +395,17 @@ impl Connection { future_into_py(py, async move { use lance_namespace::models::CreateNamespaceRequest; // Mode is now a string field - let mode_str = mode.and_then(|m| match m.to_lowercase().as_str() { - "create" => Some("Create".to_string()), - "exist_ok" => Some("ExistOk".to_string()), - "overwrite" => Some("Overwrite".to_string()), - _ => None, - }); + let mode_str = mode + .map(|m| match m.to_lowercase().as_str() { + "create" => Ok("Create".to_string()), + "exist_ok" => Ok("ExistOk".to_string()), + "overwrite" => Ok("Overwrite".to_string()), + _ => Err(PyValueError::new_err(format!( + "Invalid mode {:?}: expected one of 'create', 'exist_ok', 'overwrite'", + m + ))), + }) + .transpose()?; let request = CreateNamespaceRequest { id: Some(namespace_path), mode: mode_str, @@ -428,16 +433,26 @@ impl Connection { future_into_py(py, async move { use lance_namespace::models::DropNamespaceRequest; // Mode and Behavior are now string fields - let mode_str = mode.and_then(|m| match m.to_uppercase().as_str() { - "SKIP" => Some("Skip".to_string()), - "FAIL" => Some("Fail".to_string()), - _ => None, - }); - let behavior_str = behavior.and_then(|b| match b.to_uppercase().as_str() { - "RESTRICT" => Some("Restrict".to_string()), - "CASCADE" => Some("Cascade".to_string()), - _ => None, - }); + let mode_str = mode + .map(|m| match m.to_uppercase().as_str() { + "SKIP" => Ok("Skip".to_string()), + "FAIL" => Ok("Fail".to_string()), + _ => Err(PyValueError::new_err(format!( + "Invalid mode {:?}: expected one of 'skip', 'fail'", + m + ))), + }) + .transpose()?; + let behavior_str = behavior + .map(|b| match b.to_uppercase().as_str() { + "RESTRICT" => Ok("Restrict".to_string()), + "CASCADE" => Ok("Cascade".to_string()), + _ => Err(PyValueError::new_err(format!( + "Invalid behavior {:?}: expected one of 'restrict', 'cascade'", + b + ))), + }) + .transpose()?; let request = DropNamespaceRequest { id: Some(namespace_path), mode: mode_str,