[GitHub] [flink] XComp commented on a change in pull request #19153: [FLINK-26695][runtime] Evaluation of deleteConfigMap's return value added

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

XComp commented on a change in pull request #19153:
URL: https://github.com/apache/flink/pull/19153#discussion_r830181911



##########
File path: flink-kubernetes/src/main/java/org/apache/flink/kubernetes/kubeclient/Fabric8FlinkKubeClient.java
##########
@@ -350,14 +350,48 @@ public KubernetesLeaderElector createLeaderElector(
     @Override
     public CompletableFuture<Void> deleteConfigMapsByLabels(Map<String, String> labels) {
         return CompletableFuture.runAsync(
-                () -> this.internalClient.configMaps().withLabels(labels).delete(),
+                () -> {
+                    if (!this.internalClient.configMaps().withLabels(labels).delete()) {

Review comment:
       I couldn't find anything about the actual semantics of the 404 in the k8s API reference (see [ConfigMap DELETE for k8s 1.23](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.23/#delete-configmap-v1-core).
   
   But based on the meaning of 404 being Not Found, it's quite likely that it means that the resource didn't exist in the first place. The thing is that this is an implementation detail of the Fabric8Client implementation. Our implementation should focus on the contract presented in the interface that is used (in our case, the Fabric8 client's [Deletable](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.23/#delete-configmap-v1-core) interface. And its JavaDoc doesn't specify the semantics behind the `false` value.
   
   Relying on the contract described in the JavaDoc, we still would need to do the existence check. But I agree, that implementation-wise it's not an issue, I guess, looking into how the [fabric8's BaseOperation.delete](https://github.com/fabric8io/kubernetes-client/blob/master/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/BaseOperation.java#L485) is implemented.




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

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