Skip to content

Commit

Permalink
refactor: fine-grained catch
Browse files Browse the repository at this point in the history
  • Loading branch information
andylizf committed Dec 11, 2024
1 parent 36ca3a3 commit 8b7b075
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 7 deletions.
7 changes: 6 additions & 1 deletion sky/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -3545,8 +3545,13 @@ def storage_delete(names: List[str], all: bool, yes: bool): # pylint: disable=r
def delete_storage(name: str) -> None:
try:
sky.storage_delete(name)
except Exception as e: # pylint: disable=broad-except
except (exceptions.StorageBucketDeleteError, PermissionError) as e:
click.secho(f'Error deleting storage {name}: {e}', fg='red')
except ValueError as e:
click.secho(f'Error deleting storage {name}: {e}', fg='red')
except Exception as e: # pylint: disable=broad-except
with ux_utils.print_exception_no_traceback():
raise e

subprocess_utils.run_in_parallel(delete_storage,
names,
Expand Down
21 changes: 15 additions & 6 deletions sky/data/storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -950,18 +950,16 @@ def delete(self, store_type: Optional[StoreType] = None) -> None:
if not self.stores:
logger.info('No backing stores found. Deleting storage.')
global_user_state.remove_storage(self.name)
if store_type:
if store_type is not None:
store = self.stores[store_type]
is_sky_managed = store.is_sky_managed
# We delete a store from the cloud if it's sky managed. Else just
# remove handle and return
if is_sky_managed:
if store.is_sky_managed:
self.handle.remove_store(store)
store.delete()
# Check remaining stores - if none is sky managed, remove
# the storage from global_user_state.
delete = all(
s.is_sky_managed is False for s in self.stores.values())
delete = all(not s.is_sky_managed for s in self.stores.values())
if delete:
global_user_state.remove_storage(self.name)
else:
Expand Down Expand Up @@ -1491,6 +1489,9 @@ def _delete_s3_bucket(self, bucket_name: str) -> bool:
Returns:
bool; True if bucket was deleted, False if it was deleted externally.
Raises:
StorageBucketDeleteError: If deleting the bucket fails.
"""
# Deleting objects is very slow programatically
# (i.e. bucket.objects.all().delete() is slow).
Expand Down Expand Up @@ -1934,6 +1935,11 @@ def _delete_gcs_bucket(self, bucket_name: str) -> bool:
Returns:
bool; True if bucket was deleted, False if it was deleted externally.
Raises:
StorageBucketDeleteError: If deleting the bucket fails.
PermissionError: If the bucket is external and the user is not
allowed to delete it.
"""

with rich_utils.safe_status(
Expand Down Expand Up @@ -3096,6 +3102,9 @@ def _delete_r2_bucket(self, bucket_name: str) -> bool:
Returns:
bool; True if bucket was deleted, False if it was deleted externally.
Raises:
StorageBucketDeleteError: If deleting the bucket fails.
"""
# Deleting objects is very slow programatically
# (i.e. bucket.objects.all().delete() is slow).
Expand Down Expand Up @@ -3532,7 +3541,7 @@ def _create_cos_bucket(self,

return self.bucket

def _delete_cos_bucket(self):
def _delete_cos_bucket(self) -> None:
bucket = self.s3_resource.Bucket(self.name)
try:
bucket_versioning = self.s3_resource.BucketVersioning(self.name)
Expand Down

0 comments on commit 8b7b075

Please sign in to comment.