unitycatalog-rs icon indicating copy to clipboard operation
unitycatalog-rs copied to clipboard

feat: initial batch of proto definitions

Open roeap opened this issue 1 year ago • 4 comments

This is just a first draft of what it might look like to generate models via protobuf as discussed in the RFC.

A couple of points that may be worth discussion if this is the way this project wants to go.

  • I used buf to do the heavy lifting, thus avoiding having to write obscure protoc commands.
  • The definitions contain some protovalidate annotations, which due to the lack of an implementation do nothing for the rust world. However clients may be able to leverage. Specifically for instance a prospective web ui. As a corollary, personally I do enjoy using the connect protocol for frontend stuff quite a bit, which can be upcast to protobuf in e.g. envoy.
  • I tried following what buf considers best practices for protobuf, however this als leads to some nested / verbose types, which may not ne desired here.
  • personally I like just as a command runner. this may not be a universal opinion :). introducing some makefile-like thing will eventually be a good thing i believe ...

Happy to hear any feedback and if I should continue with this.

roeap avatar Jul 07 '24 11:07 roeap

Thank you for this @roeap! @abrassel and I chatted about this a bit on slack, I'm glad you went the route of using pbjson to generate serde compatible structs.

Question: why does it create manually implement Ser/De traits instead of deriving? Is it just protoc-gen-prost-serde's behavior?

abhiaagarwal avatar Jul 07 '24 16:07 abhiaagarwal

Question: why does it create manually implement Ser/De traits instead of deriving? Is it just protoc-gen-prost-serde's behavior?

@abhiaagarwal - yes, that's the default behavior. Reason being that the protobuf spec has an opinion on what the json representation of a protobuf message looks like. For example accepting snace_case and camelCase field names or how oneof types are represented.

As it turns out, this also makes for nicer rust types. For instance this ..

message Dependency {
  oneof dependency {
    TableDependency table = 1;
    FunctionDependency function = 2;
  }
}

gives us a nice Rust Enum, while accepting e.g.

{
  "table": ...
}

and erroring when both keys are present (which i assumed was the desired bahvior in this case :smile:)

roeap avatar Jul 07 '24 16:07 roeap

Awesome, thanks! I'll see if I can use this PR as a basis for the server code I have over at #9. I'm inclined to merge this once it's ready, I'll just let it ruminate for a couple days to see if anyone has any opinions (I'd love @abrassel to take a look at this!)

abhiaagarwal avatar Jul 07 '24 17:07 abhiaagarwal

@abhiaagarwal @roeap Thanks for pinging me! I took a look through and this looks pretty great to me. At the moment, I'm a bit myopically focused on building the rust client, but certainly being able to leverage the proto layer will make my life so much easier.

@roeap can we use these protobuf definitions compatibly with the current unitycatalog REST API? Sorry, hope that question was clear.

abrassel avatar Jul 08 '24 05:07 abrassel