mirror of
https://github.com/lancedb/lancedb.git
synced 2026-11-15 07:06:52 +00:00
docs(secrets): say why the code is as it is, not what it replaced
Comments that explained a decision by naming what came before it read as
changelog rather than rationale, and they age badly: a reader who never saw the
earlier shape learns nothing from being told it existed. This is a new surface,
so every comment here now states the property that holds today.
- `RemoteCreateFunctionRequest` said a literal "is why `secret_bindings`
silently stopped reaching the service". It now says what the struct buys:
the compiler decides what reaches the service, and a new field on the
registration request is a build error here until it is handled.
- Wire-omission comments that appealed to clients predating Secrets or
namespaces now state the rule itself -- a binding states a namespace only
when it has one, a body carries a binding list only when there is one.
- `BodyLogging::Allowed` described logging "as every request did before
Secrets existed", which is really the default: a body is diagnostic unless
the call that built it says otherwise.
- `a_version_without_bindings_keeps_the_original_wire_shape` named a shape no
longer worth referencing. Both the Rust and Python tests now name the
property they check -- absent decodes as an empty list, and an empty list
serializes back to absent.
Comments that cite the Table and Function conventions are kept. Those describe
what the code sits beside today, which is the reason a reader needs.
Tests: lancedb secret unit tests 7, `first_class_function_slice1` 9,
`first_class_function_slice2` 3, 93 Python tests across both slices.
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
6b77670e5c
commit
9d26cc42cf
@@ -311,8 +311,12 @@ def test_canonical_client_values_carry_bindings_and_no_credentials():
|
||||
assert_no_secret_values(canonical)
|
||||
|
||||
|
||||
def test_a_version_without_bindings_keeps_the_original_wire_shape():
|
||||
"""Every Function registered before Secrets existed serializes unchanged."""
|
||||
def test_a_version_without_bindings_omits_the_field_in_both_directions():
|
||||
"""A Function that binds nothing carries no ``secret_bindings`` key.
|
||||
|
||||
Absent decodes as an empty list, and an empty list serializes back to
|
||||
absent.
|
||||
"""
|
||||
value = job_result("remote_function_job.json")
|
||||
del value["secret_bindings"]
|
||||
version = FunctionVersion.from_json(json.dumps(value))
|
||||
|
||||
@@ -99,8 +99,8 @@ def test_secret_bound_udf_matches_its_shared_registration_golden():
|
||||
def test_a_namespaced_binding_records_the_path_and_the_name():
|
||||
"""A binding names the parts, so nothing has to be parsed back out.
|
||||
|
||||
A root binding carries no path at all, which is what keeps its wire shape
|
||||
identical to one written before namespaces existed.
|
||||
A root binding carries no path at all: the field is absent rather than an
|
||||
empty list, so a binding states a namespace only when it has one.
|
||||
"""
|
||||
root = EnvVarSecret(secret_name="openai-prod", env_variable="OPENAI_API_KEY")
|
||||
assert root.secret_namespace_path == []
|
||||
|
||||
@@ -503,8 +503,9 @@ pub struct FunctionArtifactRequest {
|
||||
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)]
|
||||
pub struct SecretReference {
|
||||
pub name: String,
|
||||
/// The namespace holding the Secret. Empty is the root, and is omitted from
|
||||
/// the wire so a root binding carries no trace of a feature it does not use.
|
||||
/// The namespace holding the Secret. Empty is the root, and is absent from
|
||||
/// the wire rather than sent empty: a binding states a namespace only when
|
||||
/// it has one.
|
||||
#[serde(default, skip_serializing_if = "Vec::is_empty")]
|
||||
pub namespace_path: Vec<String>,
|
||||
}
|
||||
|
||||
@@ -431,7 +431,8 @@ fn validate_dns_hostname(hostname: &str) -> Result<()> {
|
||||
/// call that has it.
|
||||
#[derive(Clone, Copy, PartialEq, Eq, Debug)]
|
||||
enum BodyLogging {
|
||||
/// Log the body at debug, as every request did before Secrets existed.
|
||||
/// Log the body at debug. The default: a request body is diagnostic unless
|
||||
/// the call that built it says otherwise.
|
||||
Allowed,
|
||||
/// Never log the body. For a request whose body is a credential.
|
||||
Suppressed,
|
||||
@@ -1323,7 +1324,7 @@ mod tests {
|
||||
}
|
||||
|
||||
/// A suppressed body is suppressed whatever the content type says, and an
|
||||
/// allowed one is logged exactly as it was before Secrets existed.
|
||||
/// allowed one is logged in full.
|
||||
#[test]
|
||||
fn test_body_logging_is_decided_by_the_caller() {
|
||||
assert_ne!(BodyLogging::Allowed, BodyLogging::Suppressed);
|
||||
|
||||
@@ -617,18 +617,16 @@ struct RemoteDropFunctionResponse {
|
||||
/// The create body: every field of [`FunctionRegistrationRequest`] except the
|
||||
/// name, which is the path identifier.
|
||||
///
|
||||
/// A struct rather than a `json!` literal naming fields one by one. The literal
|
||||
/// is why `secret_bindings` silently stopped reaching the service when the
|
||||
/// route moved -- a field added to the request type does not have to be added
|
||||
/// here to compile, it just stops being sent. This shape fails to build
|
||||
/// instead.
|
||||
/// A struct rather than a literal listing the fields, so that the compiler
|
||||
/// decides what reaches the service. A field the registration request grows is
|
||||
/// a build error here until it is handled; a literal would simply not send it.
|
||||
#[derive(serde::Serialize)]
|
||||
struct RemoteCreateFunctionRequest<'a> {
|
||||
artifact: &'a FunctionArtifactRequest,
|
||||
signature: &'a FunctionSignature,
|
||||
runtime: &'a PythonRuntimeSpec,
|
||||
/// Omitted when the Function binds nothing, so a request from a client that
|
||||
/// predates Secrets is byte-identical to one that does not use them.
|
||||
/// Absent when the Function binds nothing, so the body carries a binding
|
||||
/// list only when there is one to carry.
|
||||
#[serde(skip_serializing_if = "<[SecretBinding]>::is_empty")]
|
||||
secret_bindings: &'a [SecretBinding],
|
||||
}
|
||||
|
||||
@@ -223,9 +223,10 @@ fn an_unknown_binding_kind_is_forward_decodable() {
|
||||
);
|
||||
}
|
||||
|
||||
/// Every Function registered before Secrets existed serializes unchanged.
|
||||
/// A Function that binds nothing carries no `secret_bindings` key: absent
|
||||
/// decodes as an empty list, and an empty list serializes back to absent.
|
||||
#[test]
|
||||
fn a_version_without_bindings_keeps_the_original_wire_shape() {
|
||||
fn a_version_without_bindings_omits_the_field_in_both_directions() {
|
||||
let mut result = job_result("remote_function_job.json");
|
||||
result
|
||||
.as_object_mut()
|
||||
|
||||
Reference in New Issue
Block a user