gyfora commented on code in PR #337:
URL:
https://github.com/apache/flink-kubernetes-operator/pull/337#discussion_r938535348
##########
flink-kubernetes-operator/src/test/java/org/apache/flink/kubernetes/operator/service/NativeFlinkServiceTest.java:
##########
@@ -180,8 +186,10 @@ public void testCancelJobWithLastStateUpgradeMode() throws
Exception {
.get());
}
- @Test
- public void testTriggerSavepoint() throws Exception {
+ @ParameterizedTest
+ @MethodSource("provideSavepointFormatType")
+ public void testTriggerSavepoint(Configuration configuration,
FlinkConfigManager configManager)
+ throws Exception {
Review Comment:
These tests do not actually test that the correct savepoint format type is
used during savepointing.
We should remove these parameterized tests and simply validate that if the
user configures the savepoint type the rest api is called with that type in the
FlinkService
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/config/KubernetesOperatorConfigOptions.java:
##########
@@ -280,4 +281,12 @@ public class KubernetesOperatorConfigOptions {
.defaultValue(true)
.withDescription(
"Enables last-state fallback for savepoint upgrade
mode. When the job is not running thus savepoint cannot be triggered but HA
metadata is available for last state restore the operator can initiate the
upgrade process when the flag is enabled.");
+
+ @Documentation.Section(SECTION_DYNAMIC)
+ public static final ConfigOption<String> OPERATOR_SAVEPOINT_FORMAT_TYPE =
Review Comment:
Should this be enum type instead?
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/config/FlinkOperatorConfiguration.java:
##########
@@ -129,6 +131,14 @@ public static FlinkOperatorConfiguration
fromConfiguration(Configuration operato
String labelSelector =
operatorConfig.getString(KubernetesOperatorConfigOptions.OPERATOR_LABEL_SELECTOR);
+ SavepointFormatType savepointFormatType =
+ SavepointFormatType.valueOf(
+ operatorConfig
+ .getString(
+ KubernetesOperatorConfigOptions
+
.OPERATOR_SAVEPOINT_FORMAT_TYPE)
+ .toUpperCase());
+
Review Comment:
This setting should not be part of the `FlinkOperatorConfiguration` is it is
intended to be set on a per resource basis
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
