gyfora commented on code in PR #351: URL: https://github.com/apache/flink-kubernetes-operator/pull/351#discussion_r951468692
########## flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/crd/spec/diff/DiffResult.java: ########## @@ -0,0 +1,102 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.kubernetes.operator.crd.spec.diff; + +import org.apache.flink.annotation.Experimental; + +import lombok.NonNull; +import org.apache.commons.lang3.builder.ToStringBuilder; +import org.apache.commons.lang3.builder.ToStringStyle; + +import java.util.Collections; +import java.util.List; + +/** + * Contains a collection of the differences between two {@link Diffable} objects. + * + * <p>Inspired by: + * https://github.com/apache/commons-lang/blob/master/src/main/java/org/apache/commons/lang3/builder/DiffResult.java + */ +@Experimental +public class DiffResult<T> { + + public static final String OBJECTS_SAME_STRING = ""; + + private final List<Diff<?>> diffList; + private final T left; + private final T right; + private final DiffType type; + + DiffResult(@NonNull T left, @NonNull T right, @NonNull List<Diff<?>> diffList) { + this.left = left; + this.right = right; + this.diffList = diffList; + this.type = getSpechChangeType(diffList); + } + + public T getLeft() { + return this.left; + } + + public T getRight() { + return this.right; + } + + public List<Diff<?>> getDiffs() { + return Collections.unmodifiableList(diffList); + } + + public int getNumberOfDiffs() { + return diffList.size(); + } + + public DiffType getType() { + return type; + } Review Comment: We could use the `@Getter` for this ########## docs/content/docs/custom-resource/reference.md: ########## @@ -174,6 +174,79 @@ This page serves as a full reference for FlinkDeployment custom resource definit | LAST_STATE | Job is upgraded using any latest checkpoint or savepoint available. | | STATELESS | Job is upgraded with empty state. | +### Diff +**Class**: org.apache.flink.kubernetes.operator.crd.spec.diff.Diff + +**Description**: Contains the differences between two {@link Diffable} class fields. + + <p>Inspired by: + https://github.com/apache/commons-lang/blob/master/src/main/java/org/apache/commons/lang3/builder/Diff.java + +| Parameter | Type | Docs | +| ----------| ---- | ---- | +| serialVersionUID | long | | +| fieldName | @lombok.NonNull java.lang.String | | +| left | T | | +| right | T | | +| type | @lombok.NonNull org.apache.flink.kubernetes.operator.crd.spec.diff.DiffType | | + Review Comment: These Diff related classes should not be part of the CRD reference. We should either move them to another package or change the generator logic. ########## flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/service/FlinkService.java: ########## @@ -93,4 +93,6 @@ void triggerSavepoint( PodList getJmPodList(FlinkDeployment deployment, Configuration conf); void waitForClusterShutdown(Configuration conf); + + boolean reactiveScale(ObjectMeta meta, JobSpec jobSpec, Configuration conf); Review Comment: It could also return false by default to avoid having to implement it for non standalone clusters. ########## flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/crd/spec/diff/DiffResult.java: ########## @@ -0,0 +1,102 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.kubernetes.operator.crd.spec.diff; + +import org.apache.flink.annotation.Experimental; + +import lombok.NonNull; +import org.apache.commons.lang3.builder.ToStringBuilder; +import org.apache.commons.lang3.builder.ToStringStyle; + +import java.util.Collections; +import java.util.List; + +/** + * Contains a collection of the differences between two {@link Diffable} objects. + * + * <p>Inspired by: + * https://github.com/apache/commons-lang/blob/master/src/main/java/org/apache/commons/lang3/builder/DiffResult.java + */ +@Experimental +public class DiffResult<T> { + + public static final String OBJECTS_SAME_STRING = ""; + + private final List<Diff<?>> diffList; + private final T left; + private final T right; + private final DiffType type; + + DiffResult(@NonNull T left, @NonNull T right, @NonNull List<Diff<?>> diffList) { + this.left = left; + this.right = right; + this.diffList = diffList; + this.type = getSpechChangeType(diffList); + } + + public T getLeft() { + return this.left; + } + + public T getRight() { + return this.right; + } + + public List<Diff<?>> getDiffs() { + return Collections.unmodifiableList(diffList); + } + + public int getNumberOfDiffs() { + return diffList.size(); + } + + public DiffType getType() { + return type; + } + + @Override + public String toString() { + if (diffList.isEmpty()) { + return OBJECTS_SAME_STRING; + } + + final ToStringBuilder lhsBuilder = + new ToStringBuilder(left, ToStringStyle.SHORT_PREFIX_STYLE); + final ToStringBuilder rhsBuilder = + new ToStringBuilder(right, ToStringStyle.SHORT_PREFIX_STYLE); + + diffList.forEach( + diff -> { + lhsBuilder.append(diff.getFieldName(), diff.getLeft()); + rhsBuilder.append(diff.getFieldName(), diff.getRight()); + }); + + return String.format("%s differs from %s", lhsBuilder.build(), rhsBuilder.build()); + } + + private DiffType getSpechChangeType(List<Diff<?>> diffs) { Review Comment: should be static ########## flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/crd/spec/diff/Diff.java: ########## @@ -0,0 +1,40 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.kubernetes.operator.crd.spec.diff; + +import org.apache.flink.annotation.Experimental; + +import lombok.NonNull; +import lombok.Value; + +/** + * Contains the differences between two {@link Diffable} class fields. + * + * <p>Inspired by: + * https://github.com/apache/commons-lang/blob/master/src/main/java/org/apache/commons/lang3/builder/Diff.java + */ +@Experimental +@Value +public class Diff<T> { + private static final long serialVersionUID = 1L; + + @NonNull private final String fieldName; + private final T left; + private final T right; + @NonNull private final DiffType type; Review Comment: `private final` can be removed if `@Value` is used ########## flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/service/FlinkService.java: ########## @@ -93,4 +93,6 @@ void triggerSavepoint( PodList getJmPodList(FlinkDeployment deployment, Configuration conf); void waitForClusterShutdown(Configuration conf); + + boolean reactiveScale(ObjectMeta meta, JobSpec jobSpec, Configuration conf); Review Comment: I think we should simply call this `scale` . This operation does not to be reactive, that's basically just one way to implement this for standalone. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
