search

Arnavion / k8s-openapi

Rust definitions of the resource types in the Kubernetes client API
Apache License 2.0
379 stars 41 forks source link

Add a resource_version trait for Metadata Ty's? #60

Closed clux closed 4 years ago

clux commented 4 years ago

Hey there. I am trying to migrate over kube-rs to use your exported traits, which you've super helpfully implemented for all the k8s objects. But I have a problem for the Metadata trait.

Currently it just enforces that there is an opaque Ty type accesssible via metadata, but I need to actually read the resource_version out of metadata to build higher order absractions such as informers. And in that use case, I don't know what metadata type I am working with.

I can see that resource_version exists for both implementations of the metadata Types. Do you think this is worth exposing as a trait?

Alternatively, I am probably able to do something hacky with the serialize impl on the Metadata Tys (serialize as string, read back using serde_yaml, and look for the resource_version key).

clux commented 4 years ago

Nevermind, I think we have a solution :-)

Arnavion commented 4 years ago

Yeah, the solution you came up with is what I would've suggested too. But I can add a trait for fn resource_version(&self) and fn self_link(&self) anyway.

(Hard part is what to call it, since Metadata is already taken. Guess I can make a breaking change and rename the current one to HasMetadata or something.)

clux commented 4 years ago

Having implemented what we needed, I'm no longer convinced that a trait on your end would help our case a lot (though I appreciate it). Reflectors and Informers can only really be built on top of objects that have a .name and (optional) .namespace keys on their metadata, e.g. they actually assume they have ObjectMeta as Ty.

This also makes sense because they are designed to watch collection of Thing kinds, not a collection of ThingList kinds. That wouldn't really make sense.

Have commented my current solution in https://github.com/clux/kube-rs/issues/150#issuecomment-592988856, which works fine, and caused us to rip out like 500 lines of now unnecessary mappings between kube structs and k8s-openapi :smile:

Arnavion commented 4 years ago

Given that self_link is deprecated, and just resource_version by itself doesn't appear to be very useful as you said, there isn't much point in adding a common trait.