[GitHub] [flink-kubernetes-operator] wangyang0918 commented on pull request #9: [FLINK-26163] Refactor FlinkUtils#getEffectiveConfig into smaller pieces

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

wangyang0918 commented on pull request #9:
URL: https://github.com/apache/flink-kubernetes-operator/pull/9#issuecomment-1046798237


   > 1. Refactor `FlinkUtils#getEffectiveConfig` into smaller pieces.
   > 2. Fix the problem that the field of `replicas` in `JobManagerSpec` will not take effect.
   > 3. Fix the problem that the basic `podTemplate` in `FlinkDeploymentSpec` will not take effect if JM or TM spec does not specify `podTemplate` on their own.
   > 4. Refactor some codes in `TestUtils` and add unit test for `FlinkUtils#getEffectiveConfig`.
   
   * I am suggesting to factor out No.2 into a separate ticket.
   * Why we have the No.3 issue? IIUC, currently the pod template should work for both JobManager and TaskManager.
   * For No.1, what do you think about introducing a `FlinkConfigBuilder` just like following. It will make things easier to test.
   ```
   /** Build effective flink config from {@link FlinkDeployment} */
   public class FlinkConfigBuilder {
       private final FlinkDeployment deploy;
       private final Configuration effectiveConfig;
   
       public FlinkConfigBuilder(FlinkDeployment deploy) {
           this.deploy = deploy;
           this.effectiveConfig = FlinkUtils.loadConfiguration();
       }
   
       public FlinkConfigBuilder applyImageToConfig() {
           effectiveConfig.set(KubernetesConfigOptions.CONTAINER_IMAGE, deploy.getSpec().getImage());
           return this;
       }
   
       // Apply other fields to the config
       ... ...
   
       public Configuration build() {
           // Set some basic configuration
           final String namespace = deploy.getMetadata().getNamespace();
           final String clusterId = deploy.getMetadata().getName();
           effectiveConfig.setString(KubernetesConfigOptions.NAMESPACE, namespace);
           effectiveConfig.setString(KubernetesConfigOptions.CLUSTER_ID, clusterId);
           return effectiveConfig;
       }
   }
   ```
   
   ```
   final Configuration effectiveConfig = new FlinkConfigBuilder(flinkApp).applyImageToConfig().apply<...>.build();
   ```


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

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