tillrohrmann commented on a change in pull request #13190:
URL: https://github.com/apache/flink/pull/13190#discussion_r473709875
##########
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:
I think the idea was that the `ClusterDescriptor` will generate a
cluster id when you call any of the `deploy*` methods. I believe what is set in
the constructor of the `YarnClusterDescriptor` is fine and does not contradict
what I was saying.
If one really needs to configure the cluster id manually, then one could
extend the `ClusterDescriptor` interface accordingly.
I think that we are adding technical debt by working around broken contracts
which might be fine now but we should definitely pull this straight eventually.
----------------------------------------------------------------
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
