From 3e148ee157123eebdf583e2055234ea7f50eae9f Mon Sep 17 00:00:00 2001 From: Doyoung Kim <34902420+landscapepainter@users.noreply.github.com> Date: Thu, 25 May 2023 16:07:47 -0700 Subject: [PATCH] Revert "[UX] Optimizing error message when disabled cloud storage attempted (#1858)" This reverts commit 4849c866de987279081aafd6152c8f07f22c9918. --- sky/adaptors/cloudflare.py | 1 - sky/data/storage.py | 36 +----------------------------------- sky/global_user_state.py | 20 +------------------- sky/task.py | 11 +++++++---- 4 files changed, 9 insertions(+), 59 deletions(-) diff --git a/sky/adaptors/cloudflare.py b/sky/adaptors/cloudflare.py index 50ad747926f3..8099605983c7 100644 --- a/sky/adaptors/cloudflare.py +++ b/sky/adaptors/cloudflare.py @@ -14,7 +14,6 @@ AWS_R2_PROFILE_PATH = '~/.aws/credentials' R2_PROFILE_NAME = 'r2' _INDENT_PREFIX = ' ' -NAME = 'Cloudflare' def import_package(func): diff --git a/sky/data/storage.py b/sky/data/storage.py index 6cdd24cd0f94..5225c283e9b4 100644 --- a/sky/data/storage.py +++ b/sky/data/storage.py @@ -40,9 +40,7 @@ # Storage isn't supported yet (even though Azure is). # TODO(Doyoung): need to add clouds.CLOUDFLARE() to support # R2 to be an option as preferred store type -STORE_ENABLED_CLOUDS: List[str] = [ - str(clouds.AWS()), str(clouds.GCP()), cloudflare.NAME -] +STORE_ENABLED_CLOUDS = [clouds.AWS(), clouds.GCP()] # Maximum number of concurrent rsync upload processes _MAX_CONCURRENT_UPLOADS = 32 @@ -914,17 +912,6 @@ def _validate(self): # Validate name self.name = self.validate_name(self.name) - # Check if the storage is enabled - enabled_storage_clouds = global_user_state.get_enabled_storage_clouds() - if str(clouds.AWS()) not in enabled_storage_clouds: - with ux_utils.print_exception_no_traceback(): - raise exceptions.ResourcesUnavailableError( - 'Storage \'store: s3\' specified, but ' \ - 'AWS access is disabled. To fix, enable '\ - 'AWS by running `sky check`. More info: '\ - 'https://skypilot.readthedocs.io/en/latest/getting-started/installation.html.' # pylint: disable=line-too-long - ) - @classmethod def validate_name(cls, name) -> str: """Validates the name of the S3 store. @@ -1300,16 +1287,6 @@ def _validate(self): 'R2 Bucket should exist.') # Validate name self.name = self.validate_name(self.name) - # Check if the storage is enabled - enabled_storage_clouds = global_user_state.get_enabled_storage_clouds() - if str(clouds.GCP()) not in enabled_storage_clouds: - with ux_utils.print_exception_no_traceback(): - raise exceptions.ResourcesUnavailableError( - 'Storage \'store: gcs\' specified, but ' \ - 'GCP access is disabled. To fix, enable '\ - 'GCP by running `sky check`. '\ - 'More info: https://skypilot.readthedocs.io/en/latest/getting-started/installation.html.' # pylint: disable=line-too-long - ) @classmethod def validate_name(cls, name) -> str: @@ -1712,18 +1689,7 @@ def _validate(self): assert self.name == data_utils.split_r2_path(self.source)[0], ( 'R2 Bucket is specified as path, the name should be ' 'the same as R2 bucket.') - # Validate name self.name = S3Store.validate_name(self.name) - # Check if the storage is enabled - enabled_storage_clouds = global_user_state.get_enabled_storage_clouds() - if cloudflare.NAME not in enabled_storage_clouds: - with ux_utils.print_exception_no_traceback(): - raise exceptions.ResourcesUnavailableError( - 'Storage \'store: r2\' specified, but ' \ - 'Cloudflare R2 access is disabled. To fix, '\ - 'enable Cloudflare R2 by running `sky check`. '\ - 'More info: https://skypilot.readthedocs.io/en/latest/getting-started/installation.html.' # pylint: disable=line-too-long - ) def initialize(self): """Initializes the R2 store object on the cloud. diff --git a/sky/global_user_state.py b/sky/global_user_state.py index ca858ee45bd5..4bd6ff679121 100644 --- a/sky/global_user_state.py +++ b/sky/global_user_state.py @@ -20,10 +20,8 @@ import colorama from sky import clouds -from sky.adaptors import cloudflare -from sky.data import storage as storage_lib -from sky.utils import common_utils from sky.utils import db_utils +from sky.utils import common_utils if typing.TYPE_CHECKING: from sky import backends @@ -665,22 +663,6 @@ def get_enabled_clouds() -> List[clouds.Cloud]: return enabled_clouds -def get_enabled_storage_clouds() -> List[str]: - # This is a temporary solution until https://github.com/skypilot-org/skypilot/issues/1943 # pylint: disable=line-too-long - # is resolved by implementing separate 'enabled_storage_clouds' - enabled_clouds = get_enabled_clouds() - enabled_clouds = [str(cloud) for cloud in enabled_clouds] - - enabled_storage_clouds = [ - cloud for cloud in enabled_clouds - if cloud in storage_lib.STORE_ENABLED_CLOUDS - ] - r2_is_enabled, _ = cloudflare.check_credentials() - if r2_is_enabled: - enabled_storage_clouds.append(cloudflare.NAME) - return enabled_storage_clouds - - def set_enabled_clouds(enabled_clouds: List[str]) -> None: _DB.cursor.execute('INSERT OR REPLACE INTO config VALUES (?, ?)', (_ENABLED_CLOUDS_KEY, json.dumps(enabled_clouds))) diff --git a/sky/task.py b/sky/task.py index 377073661c8c..972a1e95e37e 100644 --- a/sky/task.py +++ b/sky/task.py @@ -686,10 +686,13 @@ def get_preferred_store_type(self) -> storage_lib.StoreType: if storage_cloud is None: # Get the first enabled cloud. backend_utils.check_public_cloud_enabled() - enabled_storage_clouds = \ - global_user_state.get_enabled_storage_clouds() - if enabled_storage_clouds: - storage_cloud = enabled_storage_clouds[0] + enabled_clouds = global_user_state.get_enabled_clouds() + + for cloud in storage_lib.STORE_ENABLED_CLOUDS: + for enabled_cloud in enabled_clouds: + if cloud.is_same_cloud(enabled_cloud): + storage_cloud = cloud + break if storage_cloud is None: raise ValueError('No available cloud to mount storage.') store_type = storage_lib.get_storetype_from_cloud(storage_cloud)