[GitHub] [flink-kubernetes-operator] gaborgsomogyi commented on a diff in pull request #434: [FLINK-29919] Support operator leader election

[GitBox <gi...@apache.org>]

gaborgsomogyi commented on code in PR #434:
URL: https://github.com/apache/flink-kubernetes-operator/pull/434#discussion_r1019246303


##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/config/FlinkOperatorConfiguration.java:
##########
@@ -60,6 +62,7 @@ public class FlinkOperatorConfiguration {
     int exceptionFieldLengthThreshold;
     int exceptionThrowableCountThreshold;
     String labelSelector;
+    LeaderElectionConfiguration leaderElectionConfiguration;

Review Comment:
   Nit: any reason why these are not private?



##########
flink-kubernetes-operator/src/test/java/org/apache/flink/kubernetes/operator/FlinkOperatorTest.java:
##########
@@ -74,4 +93,16 @@ public void testConfigurationPassedToJOSDK() {
 
         Assertions.assertThrows(IllegalStateException.class, () -> new FlinkOperator(secondConfig));
     }
+
+    @Test
+    public void testLeaderElectionConfig() {
+        var operatorConfig = new Configuration();
+        operatorConfig.set(KubernetesOperatorConfigOptions.OPERATOR_LEADER_ELECTION_ENABLED, true);
+
+        try {
+            new FlinkOperator(operatorConfig);
+        } catch (IllegalConfigurationException ice) {
+            assertTrue(ice.getMessage().startsWith("Lease name must be defined"));

Review Comment:
   Nit: Maybe we can mention either the feature (leader election) or config name. Such case one don't need to dig for conf name just copy-paste.



##########
pom.xml:
##########
@@ -69,7 +69,7 @@ under the License.
         <maven-resources-plugin.version>3.2.0</maven-resources-plugin.version>
         <git-commit-id-maven-plugin.version>5.0.0</git-commit-id-maven-plugin.version>
 
-        <operator.sdk.version>4.1.0</operator.sdk.version>
+        <operator.sdk.version>4.1.1</operator.sdk.version>

Review Comment:
   Just for my own understanding, is this needed for `KUBERNETES_NAMESPACE_SYSTEM_PROPERTY`?



-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org