The java generator produces a new for each type occurrence about kubernetes-client HOT 10 OPEN

Hey @iocanel ! Sorry for the slow response, at the moment I'm pretty busy too 🙁
To help this issue move forward what I can commit to do is to try to apply the experimental Maven Plugin: https://github.com/andreaTP/poc-class-dedup
to your kubevirt project with an "allowList" for classes to be deduplicated.

This way, we can start experimenting with the feature on a real-world use-case.
Thoughts?

from kubernetes-client.

Hi @iocanel , and thanks for opening this super interesting discussion.
I have been tinkering around those things for a while, but couldn't get convinced about one possible implementation and ended up delaying it, thanks for bumping it up!

The big show-stopper for me has been forward compatibility, imagine the following scenario.
V1:

• class1 -> class1.metadata1
• class2 -> class2.metadata2
  the 2 metadata fields are identical and we squash them into one object, so it becomes:
• class1 -> root.metadata
• class2 -> root.metadata

This is nice from the deduplication POV, code size etc. etc.
But we don't have control over the shape of metadata, as it's defined by the user, and, for the next V1 release they can add a field to metadata2 and not to metadata1 which is a fair non-breaking change.
Unfortunately, we end up being unable to "refactor" the metadata classes, causing a breaking generated code change (package names etc. etc.).

Your proposal is interesting, as it means that the user is still in control of "what to reuse", although it potentially has the same drawback explained above.

For completeness, at some point, I have prototyped a Maven plugin to do the refactoring as a separate step:
https://github.com/andreaTP/poc-class-dedup
Not sure if it still works ... but a more "configurable" and generic version of it might provide the functionality.

wdyt?

from kubernetes-client.

More thoughts ...

I think that the are three kinds of duplicates:

1. core kubernetes types
2. cross apiVersion
3. cross apiGroup (but not core).

The simplest case to deal with is 2 as in most cases we want no reuse.
What in comes to types coming from the core kubernetes model (case 1), we could possibly reuse them, as we already do with the buildable references (we use a curated list classes that we consider reusable).

Which brings us down to cross apiGroup (case 3) and beyond the known core model classes. I think that the safest thing to do is to do nothing out of the box but provide users with the ability to define what is meant to be reused via configuration.

from kubernetes-client.

What in comes to types coming from the core kubernetes model (case 1), we could possibly reuse them, as we already do with the buildable references (we use a curated list classes that we consider reusable).

This is interesting, as it's valid reasoning, but based on assumptions.
More often than not the CRD is generated by something like kubebuilder and we don't have any guarantee that the object "backing" the Json schema is the "core Kuberentes type" or a slightly modified version of it, or even something completely different but with a matching shape.

I like the idea of leaving this decision to the user though, and this is probably the deal-breaker for implementing this feature.

from kubernetes-client.

I think that when it comes to core types, it safe enough to assume that if name (e.g. metadata) matches and shape matches then reuse. I really doubt that anyone would replicate them and not reuse them instead. Again, I am not talking about any type, but a handpicked set (like we do for BuildableReferences). As long as there is a way for the user to override our defaults I think it should be fine.

from kubernetes-client.

I'm adding an example I'm aware of: https://github.com/Apicurio/apicurio-registry-operator/blob/94d81decf63b5692bde34a35d8a612caff43933f/api/v1/apicurioregistry_types.go#L291

But I agree with your reasoning @iocanel , I think that the question becomes, should we embed this functionality in the java-generator itself or try a different approach with an additional plugin?

from kubernetes-client.

But I agree with your reasoning @iocanel , I think that the question becomes, should we embed this functionality in the java-generator itself or try a different approach with an additional plugin?

I think it needs to be in the java-generator else we won't be able to use the contextual info.

from kubernetes-client.

we won't be able to use the contextual info

I think that we need only class names and shapes, and configuring it with the "original" kubernetes yaml/json seems appealing. That said, are you gonna take a stab at it?

from kubernetes-client.

I think that we need only class names and shapes, and configuring it with the "original" kubernetes yaml/json seems appealing. That said, are you gonna take a stab at it?

Unfortunately, I don't have the time needed.

from kubernetes-client.

Something interesting I encountered while playing with kubevirt. I have 17 different implementations for Secret. Not sure if all of them could be replaced by upstream Secret, but my guess is that they could.

from kubernetes-client.