mirror of
https://github.com/lancedb/lancedb.git
synced 2026-09-11 15:52:17 +00:00
refactor(secrets): name the binding field for its delivery mode, cap in Rust
`secret_bindings` read as the general record of what a Function needs, but a `Map<String, String>` keyed on environment-variable name can only ever hold an env-delivered binding: an accessor binding has no variable to key on. Naming the field for its delivery mode leaves a later mode a sibling field rather than a tagged value type, which would break a field already in the identity hash. The per-Function cap moves from `bind_secrets` to `Connection::create_function_async`, above the backend dispatch, so it holds for every language surface rather than only the one that validates first. The low-level PyO3 entry point reached the wire past the Python check; it no longer does. Also corrects a doc comment claiming registration fails when a bound Secret is absent. Existence is first answered at `add_columns`, by design. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UfmeJ533rQDnPBkMtjerV6
This commit is contained in:
co-authored by
Claude Opus 5
parent
7b29fb2f51
commit
4d09f8ce26
@@ -310,7 +310,7 @@ class FunctionVersion(_RemoteValue):
|
||||
runtime: PythonRuntimeSpec
|
||||
runtime_digest: str
|
||||
environment_digest: str
|
||||
secret_bindings: Mapping[str, str] = {}
|
||||
secret_env_bindings: Mapping[str, str] = {}
|
||||
created_at: str
|
||||
|
||||
def __call__(self, **inputs: Any) -> FunctionApplication:
|
||||
@@ -375,7 +375,7 @@ class FunctionRegistrationRequest(_RemoteValue):
|
||||
"""Stable remote registration envelope produced by :func:`udf`.
|
||||
|
||||
Credential values deliberately have no field here. The only secret-shaped
|
||||
thing a client sends is ``secret_bindings``: the name of a Secret the
|
||||
thing a client sends is ``secret_env_bindings``: the name of a Secret the
|
||||
database already holds, which the remote service resolves at execution.
|
||||
"""
|
||||
|
||||
@@ -383,7 +383,7 @@ class FunctionRegistrationRequest(_RemoteValue):
|
||||
artifact: FunctionArtifactRequest
|
||||
signature: FunctionSignature
|
||||
runtime: PythonRuntimeSpec
|
||||
secret_bindings: Mapping[str, str] = {}
|
||||
secret_env_bindings: Mapping[str, str] = {}
|
||||
|
||||
|
||||
class FunctionVersionRef(_OpenRemoteValue):
|
||||
@@ -534,12 +534,6 @@ class RefreshColumnResult(_RemoteValue):
|
||||
_FUNCTION_NAME = re.compile(r"^[A-Za-z_][A-Za-z0-9_.-]*$")
|
||||
_DECLARED_SECRET = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$")
|
||||
|
||||
MAX_FUNCTION_SECRET_BINDINGS = 16
|
||||
"""A Function binds at most this many Secrets.
|
||||
|
||||
Each bound Secret is one extra read on the launch path of every fragment, so
|
||||
the count needs a bound for the same reason a credential needs a size limit.
|
||||
"""
|
||||
_FUNCTION_BLOB_V2_TYPE = "blob_v2"
|
||||
_ARROW_EXTENSION_NAME_KEY = "ARROW:extension:name"
|
||||
_BLOB_V2_EXTENSION_NAME = "lance.blob.v2"
|
||||
@@ -1310,11 +1304,6 @@ class UdfDefinition:
|
||||
f"Function secrets must be EnvVarSecret values, not {kinds!r}; a "
|
||||
"credential value is never sent to this API"
|
||||
)
|
||||
if len(bindings) > MAX_FUNCTION_SECRET_BINDINGS:
|
||||
raise ValueError(
|
||||
f"a Function binds at most {MAX_FUNCTION_SECRET_BINDINGS} secrets, "
|
||||
f"not {len(bindings)}"
|
||||
)
|
||||
variables = [binding.env_variable for binding in bindings]
|
||||
duplicates = sorted({name for name in variables if variables.count(name) > 1})
|
||||
if duplicates:
|
||||
@@ -1334,7 +1323,7 @@ class UdfDefinition:
|
||||
if not bindings:
|
||||
return self._request
|
||||
resolved = {binding.env_variable: binding.secret for binding in bindings}
|
||||
return self._request._copy(update={"secret_bindings": resolved})
|
||||
return self._request._copy(update={"secret_env_bindings": resolved})
|
||||
|
||||
def __call__(self, *args, **kwargs):
|
||||
return self._function(*args, **kwargs)
|
||||
|
||||
@@ -110,7 +110,7 @@ def test_function_version_identity_is_immutable_and_exact():
|
||||
version = FunctionVersion.from_json(json.dumps(value))
|
||||
assert version.name == "embed"
|
||||
assert version.version == "fv_01K3EXACT"
|
||||
assert dict(version.secret_bindings) == {"HF_TOKEN": "hf-prod"}
|
||||
assert dict(version.secret_env_bindings) == {"HF_TOKEN": "hf-prod"}
|
||||
|
||||
with pytest.raises((TypeError, ValueError)):
|
||||
version.version = "fv_changed"
|
||||
@@ -299,17 +299,17 @@ def test_canonical_client_values_carry_bindings_and_no_credentials():
|
||||
json.dumps(job_result("remote_function_job.json"))
|
||||
)
|
||||
canonical = json.loads(version.to_canonical_json())
|
||||
assert canonical["secret_bindings"] == {"HF_TOKEN": "hf-prod"}
|
||||
assert canonical["secret_env_bindings"] == {"HF_TOKEN": "hf-prod"}
|
||||
assert_no_secret_values(canonical)
|
||||
|
||||
|
||||
def test_a_version_without_bindings_keeps_the_original_wire_shape():
|
||||
"""Every Function registered before Secrets existed serializes unchanged."""
|
||||
value = job_result("remote_function_job.json")
|
||||
del value["secret_bindings"]
|
||||
del value["secret_env_bindings"]
|
||||
version = FunctionVersion.from_json(json.dumps(value))
|
||||
assert dict(version.secret_bindings) == {}
|
||||
assert "secret_bindings" not in json.loads(version.to_canonical_json())
|
||||
assert dict(version.secret_env_bindings) == {}
|
||||
assert "secret_env_bindings" not in json.loads(version.to_canonical_json())
|
||||
|
||||
|
||||
class _FunctionDeclarationInner:
|
||||
|
||||
@@ -21,6 +21,7 @@ import pyarrow as pa
|
||||
import pytest
|
||||
|
||||
import lancedb
|
||||
from lancedb.background_loop import LOOP
|
||||
from lancedb.functions import (
|
||||
PythonRuntimeSpec,
|
||||
UdfDefinition,
|
||||
@@ -100,7 +101,7 @@ def test_an_unbound_request_carries_no_binding_at_all():
|
||||
whether or not a credential is later bound to it.
|
||||
"""
|
||||
unbound = json.loads(analyze_caption.registration_request.to_canonical_json())
|
||||
assert "secret_bindings" not in unbound
|
||||
assert "secret_env_bindings" not in unbound
|
||||
assert "OPENAI_API_KEY" not in json.dumps(unbound)
|
||||
|
||||
|
||||
@@ -120,7 +121,7 @@ def test_a_function_declaring_no_secret_is_registered_exactly_as_before():
|
||||
== normalize_score.registration_request.to_canonical_json()
|
||||
)
|
||||
assert (
|
||||
"secret_bindings"
|
||||
"secret_env_bindings"
|
||||
not in normalize_score.registration_request.to_canonical_json()
|
||||
)
|
||||
|
||||
@@ -144,12 +145,37 @@ def test_bindings_may_not_collide_with_plain_configuration():
|
||||
|
||||
|
||||
def test_a_function_binds_at_most_sixteen_secrets():
|
||||
"""The cap lives in Rust, so no language surface can be talked past it.
|
||||
|
||||
Registering through the typed API and hand-rolling the request envelope
|
||||
reach the same boundary, and neither reaches the wire.
|
||||
"""
|
||||
bindings = [
|
||||
EnvVarSecret(secret=f"secret-{index}", env_variable=f"TOKEN_{index}")
|
||||
for index in range(17)
|
||||
]
|
||||
with pytest.raises(ValueError, match="at most 16 secrets"):
|
||||
normalize_score.bind_secrets(bindings)
|
||||
with _mock_remote_function_catalog() as (host, state):
|
||||
db = lancedb.connect(
|
||||
"db://dev",
|
||||
api_key="fake",
|
||||
host_override=host,
|
||||
client_config={"retry_config": {"retries": 0}},
|
||||
)
|
||||
with pytest.raises(ValueError, match="at most 16 secrets"):
|
||||
db.create_function(normalize_score, secrets=bindings)
|
||||
|
||||
envelope = json.loads(normalize_score.registration_request.to_canonical_json())
|
||||
envelope["secret_env_bindings"] = {
|
||||
f"TOKEN_{index}": f"secret-{index}" for index in range(17)
|
||||
}
|
||||
|
||||
async def submit_envelope():
|
||||
return await db._conn._inner.create_function_async(json.dumps(envelope))
|
||||
|
||||
with pytest.raises(ValueError, match="at most 16 secrets"):
|
||||
LOOP.run(submit_envelope())
|
||||
|
||||
assert state["requests"] == []
|
||||
|
||||
|
||||
def test_a_credential_value_is_rejected_in_the_binding_position():
|
||||
@@ -1332,7 +1358,7 @@ def _mock_remote_function_catalog():
|
||||
"runtime": body["runtime"],
|
||||
"runtime_digest": "sha256:runtime",
|
||||
"environment_digest": "sha256:environment",
|
||||
"secret_bindings": body.get("secret_bindings", {}),
|
||||
"secret_env_bindings": body.get("secret_env_bindings", {}),
|
||||
"created_at": "2026-08-21T00:00:00Z",
|
||||
}
|
||||
response = {"job_id": "job-register"}
|
||||
@@ -1443,10 +1469,10 @@ def test_remote_registration_sends_bindings_and_never_a_credential():
|
||||
secrets=[EnvVarSecret(secret="openai-prod", env_variable="OPENAI_API_KEY")],
|
||||
)
|
||||
|
||||
assert dict(created.secret_bindings) == {"OPENAI_API_KEY": "openai-prod"}
|
||||
assert dict(created.secret_env_bindings) == {"OPENAI_API_KEY": "openai-prod"}
|
||||
path, create_request = state["requests"][0]
|
||||
assert path == "/v1/functions/create"
|
||||
assert create_request["secret_bindings"] == {"OPENAI_API_KEY": "openai-prod"}
|
||||
assert create_request["secret_env_bindings"] == {"OPENAI_API_KEY": "openai-prod"}
|
||||
# The request names a Secret and carries nothing that could be one.
|
||||
assert create_request == json.loads(
|
||||
analyze_caption.bind_secrets(
|
||||
|
||||
Reference in New Issue
Block a user