tillrohrmann commented on a change in pull request #13190:
URL: https://github.com/apache/flink/pull/13190#discussion_r473021574
##########
File path:
flink-kubernetes/src/main/java/org/apache/flink/kubernetes/cli/KubernetesSessionCli.java
##########
@@ -88,6 +88,7 @@ public Configuration getEffectiveConfiguration(String[] args)
throws CliArgsExce
private int run(String[] args) throws FlinkException, CliArgsException {
final Configuration configuration =
getEffectiveConfiguration(args);
+
KubernetesClusterClientFactory.ensureClusterIdIsSet(configuration);
Review comment:
That way we wouldn't have this clumsy call here.
##########
File path:
flink-kubernetes/src/main/java/org/apache/flink/kubernetes/cli/KubernetesSessionCli.java
##########
@@ -88,6 +88,7 @@ public Configuration getEffectiveConfiguration(String[] args)
throws CliArgsExce
private int run(String[] args) throws FlinkException, CliArgsException {
final Configuration configuration =
getEffectiveConfiguration(args);
+
KubernetesClusterClientFactory.ensureClusterIdIsSet(configuration);
Review comment:
Shouldn't we obtain the `clusterId` from the
`KubernetesClusterDescriptor`?
##########
File path:
flink-kubernetes/src/main/java/org/apache/flink/kubernetes/KubernetesClusterClientFactory.java
##########
@@ -51,10 +51,7 @@ public boolean isCompatibleWith(Configuration configuration)
{
@Override
public KubernetesClusterDescriptor
createClusterDescriptor(Configuration configuration) {
checkNotNull(configuration);
- if
(!configuration.contains(KubernetesConfigOptions.CLUSTER_ID)) {
- final String clusterId = generateClusterId();
-
configuration.setString(KubernetesConfigOptions.CLUSTER_ID, clusterId);
- }
+ ensureClusterIdIsSet(configuration);
return new KubernetesClusterDescriptor(configuration,
KubeClientFactory.fromConfiguration(configuration));
Review comment:
It is a bit unrelated. But setting a fixed cluster id for the
`ClusterDescriptor` seems wrong. That way we can ever deploy a single cluster
using a `ClusterDescriptor`. I believe that the `YarnClusterDescriptor` works
differently.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
