gyfora commented on code in PR #162:
URL:
https://github.com/apache/flink-kubernetes-operator/pull/162#discussion_r846826594
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/utils/FlinkConfigBuilder.java:
##########
@@ -241,7 +244,9 @@ private static void setResource(
isJM
? KubernetesConfigOptions.JOB_MANAGER_CPU
: KubernetesConfigOptions.TASK_MANAGER_CPU;
- effectiveConfig.setString(memoryConfigOption.key(),
resource.getMemory());
+ if (!StringUtils.isNullOrWhitespaceOnly(resource.getMemory())) {
Review Comment:
We should not allow empty settings, this should be validated instead
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/utils/FlinkConfigBuilder.java:
##########
@@ -184,8 +184,11 @@ public FlinkConfigBuilder applyJobOrSessionSpec() throws
URISyntaxException {
if (spec.getJob() != null) {
effectiveConfig.set(
DeploymentOptions.TARGET,
KubernetesDeploymentTarget.APPLICATION.getName());
- final URI uri = new URI(spec.getJob().getJarURI());
- effectiveConfig.set(PipelineOptions.JARS,
Collections.singletonList(uri.toString()));
+ if (spec.getJob().getJarURI() != null) {
+ effectiveConfig.set(
+ PipelineOptions.JARS,
+ Collections.singletonList(new
URI(spec.getJob().getJarURI()).toString()));
+ }
Review Comment:
What's the point of this change?
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/validation/DefaultValidator.java:
##########
@@ -58,6 +58,7 @@
@Override
public Optional<String> validateDeployment(FlinkDeployment deployment) {
FlinkDeploymentSpec spec = deployment.getSpec();
+ Map<String, String> effectiveConfig =
FlinkUtils.getEffectiveConfig(deployment).toMap();
Review Comment:
I think we cannot get the `getEffectiveConfig` here as that relies on the
FlinkConfigBuilder logic which in turn relies on the validation.
We should imply get the defaultConfiguration I think.
--
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]
