gyfora commented on code in PR #1195:
URL: 
https://github.com/apache/flink-kubernetes-operator/pull/1195#discussion_r3868787220


##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/kubeclient/decorators/FlinkConfMountDecorator.java:
##########
@@ -249,4 +251,20 @@ public String getFlinkConfFilename() {
     public static boolean useStandardYamlConfig(FlinkVersion flinkVersion) {
         return flinkVersion != null && 
flinkVersion.isEqualOrNewer(FlinkVersion.v2_0);
     }
+
+    /**
+     * Re-parses Map-typed options that arrive already serialized to a String 
(e.g. the autoscaler
+     * parallelism overrides stored in the CR spec) and sets them back as 
typed values.
+     */
+    public static void normalizeMapTypedOptions(Configuration source, 
Configuration target) {
+        if (!source.containsKey(PipelineOptions.PARALLELISM_OVERRIDES.key())) {

Review Comment:
   This is a bit of a strange logic to single out a config key based on the 
autoscaler logic. Would be great if the autoscaler set it correctly already



##########
flink-autoscaler/src/main/java/org/apache/flink/autoscaler/tuning/ConfigChanges.java:
##########
@@ -39,7 +39,8 @@ public class ConfigChanges {
     @Getter private final Set<String> removals = new HashSet<>();
 
     public <T> ConfigChanges addOverride(ConfigOption<T> configOption, T 
value) {
-        overrides.put(configOption.key(), 
ConfigurationUtils.convertValue(value, String.class));
+        overrides.put(
+                configOption.key(), ConfigurationUtils.convertValue(value, 
String.class, false));

Review Comment:
   instead of hardcoding false here, why don't we decide based on the flink 
version?



-- 
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]

Reply via email to