Skip to content

Commit

Permalink
error handle failure to run list blobs API from cloud_stores.py::is_d…
Browse files Browse the repository at this point in the history
…irectory()
  • Loading branch information
landscapepainter committed Jul 9, 2024
1 parent 64c770a commit 908acb4
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 19 deletions.
36 changes: 24 additions & 12 deletions sky/backends/cloud_vm_ray_backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -4498,23 +4498,35 @@ def _execute_file_mounts(self, handle: CloudVmRayResourceHandle,
continue

storage = cloud_stores.get_storage_from_path(src)
if storage.is_directory(src):
sync = storage.make_sync_dir_command(source=src,
destination=wrapped_dst)
# It is a directory so make sure it exists.
mkdir_for_wrapped_dst = f'mkdir -p {wrapped_dst}'
else:
sync = storage.make_sync_file_command(source=src,
destination=wrapped_dst)
# It is a file so make sure *its parent dir* exists.
mkdir_for_wrapped_dst = (
f'mkdir -p {os.path.dirname(wrapped_dst)}')
try:
if storage.is_directory(src):
sync_cmd = (storage.make_sync_dir_command(
source=src, destination=wrapped_dst))
# It is a directory so make sure it exists.
mkdir_for_wrapped_dst = f'mkdir -p {wrapped_dst}'
else:
sync_cmd = (storage.make_sync_file_command(
source=src, destination=wrapped_dst))
# It is a file so make sure *its parent dir* exists.
mkdir_for_wrapped_dst = (
f'mkdir -p {os.path.dirname(wrapped_dst)}')
except Exception as e:
logger.error(
f'Failed to fetch from the bucket {src!r} to '
f'remote instance at {dst!r}.\n'
'Error details: '
f'{common_utils.format_exception(e, use_bracket=True)}.')
# If 'cmd' was appended to 'symlink_commands' for this sync, we
# remove as it failed to sync.
if not dst.startswith('~/') and not dst.startswith('/tmp/'):
symlink_commands.pop()
continue

download_target_commands = [
# Ensure sync can write to wrapped_dst (e.g., '/data/').
mkdir_for_wrapped_dst,
# Both the wrapped and the symlink dir exist; sync.
sync,
sync_cmd,
]
command = ' && '.join(download_target_commands)
# dst is only used for message printing.
Expand Down
37 changes: 30 additions & 7 deletions sky/cloud_stores.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import subprocess
import urllib.parse

from sky import sky_logging
from sky.adaptors import aws
from sky.adaptors import azure
from sky.adaptors import cloudflare
Expand All @@ -19,6 +20,8 @@
from sky.data import data_utils
from sky.data.data_utils import Rclone

logger = sky_logging.init_logger(__name__)


class CloudStorage:
"""Interface for a cloud object store."""
Expand Down Expand Up @@ -189,6 +192,18 @@ def is_directory(self, url: str) -> bool:
In cloud object stores, a "directory" refers to a regular object whose
name is a prefix of other objects.
Args:
url: Endpoint url of the container/blob.
Returns:
True if the url is an endpoint of a directory and False if it
is a blob(file).
Raises:
azure.core.exceptions.HttpResponseError: If the user's Azure
Azure account does not have sufficient IAM role for the given
storage account.
"""
# split the url using split_az_path
storage_account_name, container_name, path = data_utils.split_az_path(
Expand All @@ -204,13 +219,21 @@ def is_directory(self, url: str) -> bool:
container_client = data_utils.create_az_client(
client_type='container', container_url=container_url)
num_objects = 0
for blob in container_client.list_blobs(name_starts_with=path):
if blob.name == path:
return False
num_objects += 1
if num_objects > 1:
return True

try:
for blob in container_client.list_blobs(name_starts_with=path):
if blob.name == path:
return False
num_objects += 1
if num_objects > 1:
return True
except azure.exceptions().HttpResponseError as e:
if 'AuthorizationPermissionMismatch' in str(e):
logger.error('Failed to list blobs in container '
f'{container_url!r}. This implies insufficient '
'permissions for storage account '
f'{storage_account_name!r}. Please check your '
f'Azure account IAM roles.')
raise
# A directory with few or no items
return True

Expand Down

0 comments on commit 908acb4

Please sign in to comment.