1996fanrui commented on code in PR #633:
URL:
https://github.com/apache/flink-kubernetes-operator/pull/633#discussion_r1266709748
##########
flink-kubernetes-operator-autoscaler/src/main/java/org/apache/flink/kubernetes/operator/autoscaler/ScalingExecutor.java:
##########
@@ -230,28 +222,24 @@ private Map<JobVertexID, ScalingSummary>
computeScalingSummary(
return out;
}
- private void setVertexParallelismOverrides(
- AbstractFlinkResource<?, ?> resource,
+ private static Map<String, String> getVertexParallelismOverrides(
Map<JobVertexID, Map<ScalingMetric, EvaluatedScalingMetric>>
evaluatedMetrics,
Map<JobVertexID, ScalingSummary> summaries) {
- var flinkConf =
Configuration.fromMap(resource.getSpec().getFlinkConfiguration());
var overrides = new HashMap<String, String>();
Review Comment:
nit: The first commit updated the return and `var overrides ` type from
`Map<String, String>` to `Map<JobVertexID, Integer>`, and the second commit
updated to `Map<String, String>`.
If we want to keep the `Map<String, String>` in the end, could we don't
change it?
##########
flink-kubernetes-operator-autoscaler/src/main/java/org/apache/flink/kubernetes/operator/autoscaler/ScalingExecutor.java:
##########
@@ -112,15 +110,9 @@ public boolean scaleResource(
return false;
}
- setVertexParallelismOverrides(resource, evaluatedMetrics,
scalingSummaries);
- KubernetesClientUtils.applyToStoredCr(
- kubernetesClient,
Review Comment:
`kubernetesClient` isn't used after this PR, it's better to removing it.
--
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]
