HasMetadataOperation createVisitableBuilder

[shawkins]

I added that method for edit operations - but I can see that it's possibly not needed given we should be able to lookup the Handler does that seem reasonable? Related to this there are apiVersion members/methods on both BaseOperation and HasMetadataOperation - should those be consolidated?

I'm also thinking it may not be a good idea to use the builder for cloning - just in case there are issues, like the one seen with CustomResource cloning, that would lead to really subtle bugs.

@rohanKanojia @manusa WDYT?

[manusa]

Context #3070

[manusa]

I added that method for edit operations

I'm a little bit lost here.

Related to this there are apiVersion members/methods on both BaseOperation and HasMetadataOperation - should those be consolidated?

I only see the apiVersion member in BaseOperation:

kubernetes-client/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/base/BaseOperation.java

Line 109 in 5751f31

protected String apiVersion;

I can't find that in the HasMetadataOperation class. Maybe you are referring to the static methods in the HasMetadata class? If this is the case, I don't think they can be consolidated. However, I'm a bit confused about the purpose of that field.

I'm also thinking it may not be a good idea to use the builder for cloning

I think that, although it's a costly operation, we should probably use a serialize/deserialize (or convert to Map/Array+Map and back via Jackson) to stay on the safe side and avoid side-effects.

[shawkins]

I'm a little bit lost here.

kubernetes-client/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/base/HasMetadataOperation.java

Line 81 in 289a1ec

protected VisitableBuilder<T, ?> createVisitableBuilder(T item) {

Used in

kubernetes-client/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/base/HasMetadataOperation.java

Line 89 in 289a1ec

return patch(null, clone, createVisitableBuilder(item).accept(visitors).build(), false);

and

kubernetes-client/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/base/HasMetadataOperation.java

Line 60 in 289a1ec

return createVisitableBuilder(item).build();

It sounds like we're in agreement to remove that latter usage.

In looking at replacing the create method with handler logic, I see

kubernetes-client/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/base/OperationSupport.java

Line 124 in e1cc904

public String getAPIVersion() {

(above I mistakenly said HasMetadataOperation).

It appears to mean just the version.

and

kubernetes-client/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/base/BaseOperation.java

Line 951 in e1cc904

public String getApiVersion() {

That one only appears to be set by CustomResourceOperationsImpl and means the group + version.