SameerMesiah97 commented on code in PR #62424:
URL: https://github.com/apache/airflow/pull/62424#discussion_r2850354392


##########
providers/google/src/airflow/providers/google/cloud/hooks/gcs.py:
##########
@@ -705,22 +705,30 @@ def is_older_than(self, bucket_name: str, object_name: 
str, seconds: int) -> boo
                 return True
         return False
 
-    def delete(self, bucket_name: str, object_name: str) -> None:
+    def delete(self, bucket_name: str, object_name: str, ignore_error: bool = 
False) -> None:
         """
         Delete an object from the bucket.
 
         :param bucket_name: name of the bucket, where the object resides
         :param object_name: name of the object to delete
+        :param ignore_error: (Optional) whether to ignore NotFound exceptions. 
Default: False
         """
+        on_error = None
+        if ignore_error:
+            on_error = lambda blob: None
         client = self.get_conn()
         bucket = client.bucket(bucket_name)
         blob = bucket.blob(blob_name=object_name)
-        blob.delete()
-        get_hook_lineage_collector().add_input_asset(
-            context=self, scheme="gs", asset_kwargs={"bucket": bucket.name, 
"key": blob.name}
-        )
-
-        self.log.info("Blob %s deleted.", object_name)
+        try:
+            bucket.delete_blobs([blob], on_error=on_error)
+            get_hook_lineage_collector().add_input_asset(
+                context=self, scheme="gs", asset_kwargs={"bucket": 
bucket.name, "key": blob.name}
+            )
+            if not ignore_error:
+                self.log.info("Blob %s deleted.", object_name)
+        except NotFound:
+            self.log.warning("Blob %s in bucket %s does not exist.", 
blob.name, bucket.name)
+            raise

Review Comment:
   The logic here is sound and I do not see any breaking changes. But I do feel 
a bit too complicated for what it is trying to do. Is there any reason we can't 
keep this function as is (including `blob.delete()`), wrap it in a try/except 
block, raise when `ignore_error=False` and just log when `ignore_error=True`. 
It seems like this is increasing cognitive load without any real benefit.



-- 
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]

Reply via email to