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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
