Fix Sky Storage Delete more than 256 Items/folders + Bulk Deletion Te…

…sts (skypilot-org#1285)

* fix

* Add romil's suggestions

* Add bulk deletion tests

* ok

* Fix

sky/data/storage.py

@@ -1237,25 +1237,20 @@ def _delete_gcs_bucket(self, bucket_name: str) -> None:
           bucket_name: str; Name of bucket
         """
         try:
-            bucket = self.client.get_bucket(bucket_name)
+            self.client.get_bucket(bucket_name)
         except gcp.forbidden_exception() as e:
             # Try public bucket to see if bucket exists
             with ux_utils.print_exception_no_traceback():
                 raise PermissionError(
                     'External Bucket detected. User not allowed to delete '
                     'external bucket.') from e
 
-        num_files = subprocess.check_output(
-            f'gsutil du gs://{bucket_name} | wc -l', shell=True)
-        num_files = int(num_files)
-
         try:
             with backend_utils.safe_console_status(
                     f'[bold cyan]Deleting [green]bucket {bucket_name}'):
-                if num_files >= _GCS_RM_MAX_OBJS:
-                    remove_obj_command = f'gsutil -m rm -a gs://{bucket_name}/*'
-                    subprocess.check_output(remove_obj_command.split(' '))
-                bucket.delete(force=True)
+                remove_obj_command = ('gsutil -m rm -r'
+                                      f' gs://{bucket_name}')
+                subprocess.check_output(remove_obj_command.split(' '))
         except subprocess.CalledProcessError as e:
             logger.error(e.output)
             with ux_utils.print_exception_no_traceback():

tests/test_smoke.py

@@ -955,6 +955,29 @@ def test_new_bucket_creation_and_deletion(self, tmp_local_storage_obj,
         out = subprocess.check_output(['sky', 'storage', 'ls'])
         assert tmp_local_storage_obj.name not in out.decode('utf-8')
 
+    @pytest.mark.parametrize(
+        'store_type', [storage_lib.StoreType.S3, storage_lib.StoreType.GCS])
+    def test_bucket_bulk_deletion(self, store_type):
+        # Create a temp folder with over 256 files and folders, upload
+        # files and folders to a new bucket, then delete bucket.
+        with tempfile.TemporaryDirectory() as tmpdir:
+            subprocess.check_output(f'mkdir -p {tmpdir}/folder{{000..255}}',
+                                    shell=True)
+            subprocess.check_output(f'touch {tmpdir}/test{{000..255}}.txt',
+                                    shell=True)
+            subprocess.check_output(
+                f'touch {tmpdir}/folder{{000..255}}/test.txt', shell=True)
+
+            timestamp = str(time.time()).replace('.', '')
+            store_obj = storage_lib.Storage(name=f'sky-test-{timestamp}',
+                                            source=tmpdir)
+            store_obj.add_store(store_type)
+
+        subprocess.check_output(['sky', 'storage', 'delete', store_obj.name])
+
+        output = subprocess.check_output(['sky', 'storage', 'ls'])
+        assert store_obj.name not in output.decode('utf-8')
+
     @pytest.mark.parametrize(
         'tmp_public_storage_obj, store_type',
         [('s3://tcga-2-open', storage_lib.StoreType.S3),