bazelbuild / starlark

Starlark Language
Apache License 2.0
2.38k stars 158 forks source link

Can proto.encode_text allow and ignore None values in the struct? #239

Open ofk-goog opened 1 year ago

ofk-goog commented 1 year ago

tl;dr: The result of proto.encode_text(struct(foo="hello", bar=None)) should be "{foo: "hello"} rather than an error. Otherwise I need to use hasattr a whole lot, which seems bad.

I'm ducktyping a proto in Starlark as part of a conversion to ITS that will likely be mandated all over the googlesphere, and I've hit this annoyance. My top-level message has an optional embedded message field, which has a required field. I need the values accessible in both Starlark and in C++, and wish to use proto.encode_text to avoid needing to maintain separate Starlark and textproto versions of each config file.

message SadLegacyProto {
  required string token = 1;
}
message MyConfigProto {
  optional SadLegacyProto legacy = 1;
  optional string other_data = 2;
}

The only way to represent the absence of the legacy field now is to omit it from my starlark struct, as in my_duck_proto = struct(other_data="blah"). The proto.encode_text of this looks like '{other_data: "blah"}' which is legal textproto. My annoyance with this is I have to check if hasattr(my_duck_proto, "legacy") instead of if my_duck_proto.legacy == None. Which goes against the python style guide, which we're supposed to be mostly using for Starlark.

Why other representations don't work: If I do proto.encode_text(struct(other_data="blah", legacy=struct())) then I get '{other_data: "blah" legacy {}} which can't be parsed because the required field is missing. Similarly proto.encode_text(struct(other_data="blah", legacy=struct(token=""))), I get {other_data: "blah" legacy {token: ""}} which is also an error.

Would this be okay?