Cleanup now-unused delete_files and list_folder_contents (#489)

These have been used in the old repo cleanup code. This has now been replaced by deleting files one-by-one, which is the only operation that GCS actually supports, and which the official GCS client library is also doing behind the scenes.

Author: Swatinem

shared/api_archive/archive.py

@@ -158,13 +158,6 @@ def delete_file(self, path: str) -> None:
         """
         self.storage.delete_file(self.root, path)
 
-    @sentry_sdk.trace
-    def delete_files(self, paths: list[str]) -> None:
-        """
-        Generic method to delete files from the archive.
-        """
-        self.storage.delete_files(bucket_name=self.root, paths=paths)
-
     def read_chunks(self, commit_sha: str) -> str:
         """
         Convenience method to read a chunks file from the archive.

shared/storage/aws.py

@@ -146,54 +146,3 @@ def delete_file(self, bucket_name, path):
             return True
         except ClientError:
             raise
-
-    def delete_files(self, bucket_name: str, paths: list[str]) -> list[bool]:
-        """Batch deletes a list of files from a given bucket
-
-        Note:
-            When trying to delete a file that doesnt exists, AWS SDK will
-            return 'true' as if it was deleted.
-
-        Args:
-            bucket_name (str): The name of the bucket for the file lives
-            paths (list): A list of the paths to be deletes (default: {[]})
-
-        Raises:
-            NotImplementedError: If the current instance did not implement this method
-
-        Returns:
-            list: A list of booleans, where each result indicates whether that file was deleted
-                successfully
-        """
-        objects_to_delete = {"Objects": [{"Key": key} for key in paths]}
-        try:
-            response = self.storage_client.delete_objects(
-                Bucket=bucket_name, Delete=objects_to_delete
-            )
-        except ClientError:
-            raise
-        deletes = [error.get("Key") for error in response.get("Deleted")]
-        return [key in deletes for key in paths]
-
-    def list_folder_contents(self, bucket_name, prefix=None, recursive=True):
-        """List the contents of a specific folder
-
-        Args:
-            bucket_name (str): The name of the bucket for the file lives
-            prefix: The prefix of the files to be listed (default: {None})
-            recursive: Whether the listing should be recursive (default: {True})
-
-        Raises:
-            NotImplementedError: If the current instance did not implement this method
-        """
-        try:
-            response = self.storage_client.list_objects(
-                Bucket=bucket_name, Prefix=prefix
-            )
-        except ClientError:
-            raise
-        contents = response.get("Contents")
-        return [
-            {"name": content.get("Key"), "size": content.get("Size")}
-            for content in contents
-        ]

shared/storage/base.py

@@ -96,33 +96,3 @@ def delete_file(self, bucket_name, path):
             bool: True if the deletion was succesful
         """
         raise NotImplementedError()
-
-    def delete_files(self, bucket_name: str, paths: list[str]) -> list[bool]:
-        """Batch deletes a list of files from a given bucket
-            (what happens to the files that don't exist?)
-
-        Args:
-            bucket_name (str): The name of the bucket for the file lives
-            paths (list): A list of the paths to be deletes (default: {[]})
-
-        Raises:
-            NotImplementedError: If the current instance did not implement this method
-
-        Returns:
-            list: A list of booleans, where each result indicates whether that file was deleted
-                successfully
-        """
-        raise NotImplementedError()
-
-    def list_folder_contents(self, bucket_name, prefix=None, recursive=True):
-        """List the contents of a specific folder
-
-        Args:
-            bucket_name (str): The name of the bucket for the file lives
-            prefix: The prefix of the files to be listed (default: {None})
-            recursive: Whether the listing should be recursive (default: {True})
-
-        Raises:
-            NotImplementedError: If the current instance did not implement this method
-        """
-        raise NotImplementedError()

shared/storage/fallback.py

@@ -88,35 +88,3 @@ def delete_file(self, bucket_name, path):
         first_deletion = self.main_service.delete_file(bucket_name, path)
         second_deletion = self.fallback_service.delete_file(bucket_name, path)
         return first_deletion and second_deletion
-
-    def delete_files(self, bucket_name: str, paths: list[str]) -> list[bool]:
-        """Batch deletes a list of files from a given bucket
-            (what happens to the files that don't exist?)
-
-        Args:
-            bucket_name (str): The name of the bucket for the file lives
-            paths (list): A list of the paths to be deletes (default: {[]})
-
-        Raises:
-            NotImplementedError: If the current instance did not implement this method
-
-        Returns:
-            list: A list of booleans, where each result indicates whether that file was deleted
-                successfully
-        """
-        first_results = self.main_service.delete_files(bucket_name, paths)
-        second_results = self.fallback_service.delete_files(bucket_name, paths)
-        return [f and s for f, s in zip(first_results, second_results)]
-
-    def list_folder_contents(self, bucket_name, prefix=None, recursive=True):
-        """List the contents of a specific folder
-
-        Args:
-            bucket_name (str): The name of the bucket for the file lives
-            prefix: The prefix of the files to be listed (default: {None})
-            recursive: Whether the listing should be recursive (default: {True})
-
-        Raises:
-            NotImplementedError: If the current instance did not implement this method
-        """
-        return self.main_service.list_folder_contents(bucket_name, prefix, recursive)

shared/storage/gcp.py

@@ -142,37 +142,3 @@ def delete_file(self, bucket_name: str, path: str) -> bool:
         except google.cloud.exceptions.NotFound:
             raise FileNotInStorageError(f"File {path} does not exist in {bucket_name}")
         return True
-
-    def delete_files(self, bucket_name: str, paths: list[str]) -> list[bool]:
-        """Batch deletes a list of files from a given bucket
-            (what happens to the files that don't exist?)
-
-        Args:
-            bucket_name (str): The name of the bucket for the file lives
-            paths (list): A list of the paths to be deletes (default: {[]})
-
-        Returns:
-            list: A list of booleans, where each result indicates whether that file was deleted
-                successfully
-        """
-        bucket = self.storage_client.bucket(bucket_name)
-        blobs = [bucket.blob(path) for path in paths]
-        blobs_errored: set[storage.Blob] = set()
-        bucket.delete_blobs(blobs, on_error=blobs_errored.add)
-        return [b not in blobs_errored for b in blobs]
-
-    def list_folder_contents(self, bucket_name: str, prefix=None, recursive=True):
-        """List the contents of a specific folder
-
-        Attention: google ignores the `recursive` param
-
-        Args:
-            bucket_name (str): The name of the bucket for the file lives
-            prefix: The prefix of the files to be listed (default: {None})
-            recursive: Whether the listing should be recursive (default: {True})
-        """
-        assert recursive
-        bucket = self.storage_client.bucket(bucket_name)
-        return (
-            {"name": b.name, "size": b.size} for b in bucket.list_blobs(prefix=prefix)
-        )

shared/storage/memory.py

@@ -122,45 +122,3 @@ def delete_file(self, bucket_name, path):
         except KeyError:
             raise FileNotInStorageError()
         return True
-
-    def delete_files(self, bucket_name: str, paths: list[str]) -> list[bool]:
-        """Batch deletes a list of files from a given bucket
-            (what happens to the files that don't exist?)
-
-        Args:
-            bucket_name (str): The name of the bucket for the file lives
-            paths (list): A list of the paths to be deletes (default: {[]})
-
-        Raises:
-            NotImplementedError: If the current instance did not implement this method
-
-        Returns:
-            list: A list of booleans, where each result indicates whether that file was deleted
-                successfully
-        """
-        results = []
-        for path in paths:
-            try:
-                results.append(self.delete_file(bucket_name, path))
-            except FileNotInStorageError:
-                results.append(False)
-        return results
-
-    def list_folder_contents(self, bucket_name, prefix=None, recursive=True):
-        """List the contents of a specific folder
-
-        Args:
-            bucket_name (str): The name of the bucket for the file lives
-            prefix: The prefix of the files to be listed (default: {None})
-            recursive: Whether the listing should be recursive (default: {True})
-
-        Raises:
-            NotImplementedError: If the current instance did not implement this method
-        """
-        res = [
-            {"name": key, "size": len(self.storage[bucket_name][key].decode())}
-            for key in self.storage[bucket_name]
-            if prefix is None or key.startswith(prefix)
-        ]
-
-        return res

shared/storage/minio.py

@@ -3,7 +3,6 @@
 import logging
 import os
 import shutil
-import sys
 import tempfile
 from io import BytesIO
 from typing import BinaryIO, overload
@@ -15,7 +14,6 @@
     EnvMinioProvider,
     IamAwsProvider,
 )
-from minio.deleteobjects import DeleteObject
 from minio.error import MinioException, S3Error
 
 from shared.storage.base import CHUNK_SIZE, BaseStorageService
@@ -240,27 +238,3 @@ def delete_file(self, bucket_name: str, path: str) -> bool:
             return True
         except MinioException:
             raise
-
-    def delete_files(self, bucket_name: str, paths: list[str]) -> list[bool]:
-        try:
-            for del_err in self.minio_client.remove_objects(
-                bucket_name, [DeleteObject(path) for path in paths]
-            ):
-                print("Deletion error: {}".format(del_err))  # noqa: T201
-            return [True] * len(paths)
-        except MinioException:
-            raise
-
-    def list_folder_contents(self, bucket_name, prefix=None, recursive=True):
-        return (
-            self.object_to_dict(b)
-            for b in self.minio_client.list_objects(bucket_name, prefix, recursive)
-        )
-
-    def object_to_dict(self, obj):
-        return {"name": obj.object_name, "size": obj.size}
-
-    # TODO remove this function -- just using it for output during testing.
-    def write(self, string, silence=False):
-        if not silence:
-            sys.stdout.write((string or "") + "\n")

shared/storage/new_minio.py

@@ -2,7 +2,6 @@
 import importlib.metadata
 import json
 import logging
-import sys
 from io import BytesIO
 from typing import IO, BinaryIO, Tuple, cast, overload
 
@@ -14,7 +13,6 @@
     EnvMinioProvider,
     IamAwsProvider,
 )
-from minio.deleteobjects import DeleteObject
 from minio.error import MinioException, S3Error
 from minio.helpers import ObjectWriteResult
 from urllib3 import HTTPResponse
@@ -304,27 +302,3 @@ def delete_file(self, bucket_name, url):
             return True
         except MinioException:
             raise
-
-    def delete_files(self, bucket_name, urls=[]):
-        try:
-            for del_err in self.minio_client.remove_objects(
-                bucket_name, [DeleteObject(url) for url in urls]
-            ):
-                print("Deletion error: {}".format(del_err))  # noqa: T201
-            return [True] * len(urls)
-        except MinioException:
-            raise
-
-    def list_folder_contents(self, bucket_name, prefix=None, recursive=True):
-        return (
-            self.object_to_dict(b)
-            for b in self.minio_client.list_objects(bucket_name, prefix, recursive)
-        )
-
-    def object_to_dict(self, obj):
-        return {"name": obj.object_name, "size": obj.size}
-
-    # TODO remove this function -- just using it for output during testing.
-    def write(self, string, silence=False):
-        if not silence:
-            sys.stdout.write((string or "") + "\n")