You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/04/25 16:21:38 UTC

[GitHub] [flink-kubernetes-operator] bgeng777 commented on a diff in pull request #177: [FLINK-27303][FLINK-27309] Introduce FlinkConfigManager for efficient config management

bgeng777 commented on code in PR #177:
URL: https://github.com/apache/flink-kubernetes-operator/pull/177#discussion_r857809206


##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/reconciler/deployment/AbstractDeploymentReconciler.java:
##########
@@ -39,27 +39,22 @@ public abstract class AbstractDeploymentReconciler implements Reconciler<FlinkDe
 
     private static final Logger LOG = LoggerFactory.getLogger(AbstractDeploymentReconciler.class);
 
-    protected final FlinkOperatorConfiguration operatorConfiguration;
+    protected final FlinkConfigManager configManager;
     protected final KubernetesClient kubernetesClient;
     protected final FlinkService flinkService;
-    protected final Configuration defaultConfig;
 
     public AbstractDeploymentReconciler(
             KubernetesClient kubernetesClient,
             FlinkService flinkService,
-            FlinkOperatorConfiguration operatorConfiguration,
-            Configuration defaultConfig) {
-
+            FlinkConfigManager configManager) {
         this.kubernetesClient = kubernetesClient;
         this.flinkService = flinkService;
-        this.operatorConfiguration = operatorConfiguration;
-        this.defaultConfig = defaultConfig;
+        this.configManager = configManager;
     }
 
     @Override
     public DeleteControl cleanup(FlinkDeployment flinkApp, Context context) {
-        Configuration effectiveConfig = FlinkUtils.getEffectiveConfig(flinkApp, defaultConfig);
-        return shutdownAndDelete(flinkApp, effectiveConfig);
+        return shutdownAndDelete(flinkApp, configManager.getObserveConfig(flinkApp));

Review Comment:
   The older code builds config based on current `flinkApp` but IIUC, here `getObserveConfig`  will use `lastReconciledSpec` or `lastStableSpec`. Is this change on purpose?



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