-
Notifications
You must be signed in to change notification settings - Fork 14
Reducing common structs types or https://github.com/kube-rs/gateway-api-rs/issues/38 #147
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Dawid Nowak <[email protected]>
Signed-off-by: Dawid Nowak <[email protected]>
ea15d03
to
a974d67
Compare
Signed-off-by: Dawid Nowak <[email protected]>
a974d67
to
9958589
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First of all thank you for the PR. The time you put into this solution is very much appreciated!
I've done a high level review, and I see how the process works and the output, and it's impressive. I'll do a in-depth review soon but first I wanted to start off with some questions and comments regarding this approach from a couple dimensions:
UX
Obviously not having multiple types that do the same thing is a huge improvement, so that's great. In this draft however, users would currently have to use the "processed" feature/module. Ideally the users of this crate would have to do nothing special, nor have to understand any semantics of our crate that relate to how we generate code.
What are your thoughts on the feasibility of having these generators interleave the generated code with the code otherwise generated by kopium
? That is to say, can we avoid the new processed
module (and feature) and emit common_types.rs
and the others right next to the APIs they belong to? 🤔
Maintainability & Kopium Interactions
Overall while I think the type-reducer
has its complexities, those complexities are natural given the kind of work we're trying to do here. In my brief pass I didn't notice anything that stood out as an obvious workaround or anything like that. In fact, I was very pleased to see it's all built on top of a solid foundation on the syn crate, which is great.
I am curious though, after you've gone through the exercise of building this out: Do you have any new thoughts on the interaction with kopium
here? How much of this work might actually be applicable there? Curious as to your thoughts 🤔
cc @clux as you may find this one interesting
Thanks for the kind words. Let me answer your questions: UX Initially, I created a lot of types manually to handle duplication between the HTTP and GRPC Routes. With "processed" types I was able to automate that part somewhat. As a result, I was able to (re)move and refactor some duplicated code. It is very doable to emit "common" types and keep them side by side with "standard" types. In this approach I think we would need to provide a whole bunch of converters from "standard" types into "common" types. This would be very much akin to my original approach to implementing Kubvernor. I think from the developer perspective such an approach would be confusing because the developers would need to keep selecting which type to use at a given moment in time and keep on converting from one type to another. Maintainability & Kopium Interactions When one investigates the Go implementation, it is clear that there is a lot of code reuse there. GRPCRoute is dependent on types from HTTPRoute for example and there is shared_types module with type definitions shared among all CRDs. This reuse disappears when yaml versions of the CRDs are generated. All information on how the types are reused is erased which makes it super hard for Kopium. I think that if yaml/OpenAPI definitions relied on OpenAPI references to reuse shared definitions then it would be much easier for Kopium to generate Rust code very similar to the one in "processed" APIs. Indeed, this PR probably wouldn't even be necessary. I don't know enough of the Kubernetes/Gateway API process to speculate whether it would be possible to change how yaml versions are generated. |
This approach aims to simplify the types and eliminate redundant ones. The approach works as follows:
We follow this process in phases:
Problems: