From 2d9a3b20b3812b2aaad82754a44e6e6c58f65114 Mon Sep 17 00:00:00 2001 From: Doyoung Kim Date: Wed, 12 Apr 2023 21:48:57 -0700 Subject: [PATCH 01/34] ux_optimization_disabled_clouds --- sky/data/data_utils.py | 8 ++++++++ sky/data/storage.py | 6 ++++++ sky/exceptions.py | 5 +++++ sky/global_user_state.py | 7 +++++++ sky/task.py | 22 ++++++++++++++++++++++ 5 files changed, 48 insertions(+) diff --git a/sky/data/data_utils.py b/sky/data/data_utils.py index b1048c3ec83..c51fc93af38 100644 --- a/sky/data/data_utils.py +++ b/sky/data/data_utils.py @@ -111,6 +111,14 @@ def is_cloud_store_url(url): return result.netloc +def get_cloud_store_type(url): + result = urllib.parse.urlsplit(url) + store_type = result.scheme + if store_type == 'gs': + store_type = 'gcs' + return store_type + + def _group_files_by_dir( source_list: List[str]) -> Tuple[Dict[str, List[str]], List[str]]: """Groups a list of paths based on their directory diff --git a/sky/data/storage.py b/sky/data/storage.py index 10d087520d2..3320e286b78 100644 --- a/sky/data/storage.py +++ b/sky/data/storage.py @@ -41,6 +41,12 @@ # R2 to be an option as preferred store type STORE_ENABLED_CLOUDS = [clouds.AWS(), clouds.GCP()] +STORE_TYPE_TO_CLOUD_TYPE = { + 's3' : 'AWS', + 'gcs' : 'GCP', + 'r2' : 'Cloudflare' +} + # Maximum number of concurrent rsync upload processes _MAX_CONCURRENT_UPLOADS = 32 diff --git a/sky/exceptions.py b/sky/exceptions.py index 0211cb81efa..6075f5134a3 100644 --- a/sky/exceptions.py +++ b/sky/exceptions.py @@ -209,3 +209,8 @@ class ClusterOwnerIdentityMismatchError(Exception): class NoCloudAccessError(Exception): """Raised when all clouds are disabled.""" pass + + +class CloudDisabledError(Exception): + """Raised when attempted cloud is disabled""" + pass \ No newline at end of file diff --git a/sky/global_user_state.py b/sky/global_user_state.py index 7570cfe41ce..5ea5a9c54d4 100644 --- a/sky/global_user_state.py +++ b/sky/global_user_state.py @@ -22,6 +22,7 @@ from sky import clouds from sky.utils import db_utils from sky.utils import common_utils +from sky.adaptors import cloudflare if typing.TYPE_CHECKING: from sky import backends @@ -658,6 +659,12 @@ def get_enabled_clouds() -> List[clouds.Cloud]: cloud = clouds.CLOUD_REGISTRY.from_str(c) if cloud is not None: enabled_clouds.append(cloud) + # Currently, 'clouds' only support cloud types with + # computing instances. Hence, the following is a + # temporary solution to support R2 + if os.path.exists( + os.path.expanduser(cloudflare.ACCOUNT_ID_PATH)): + enabled_clouds.append('Cloudflare') return enabled_clouds diff --git a/sky/task.py b/sky/task.py index 0710d73c007..b1a1d7bb88f 100644 --- a/sky/task.py +++ b/sky/task.py @@ -282,6 +282,17 @@ def from_yaml(yaml_path: str) -> 'Task': for dst_path, src in file_mounts.items(): # Check if it is str path if isinstance(src, str): + if data_utils.is_cloud_store_url(src): + store_type = data_utils.get_cloud_store_type(src) + cloud_type = storage_lib.STORE_TYPE_TO_CLOUD_TYPE[store_type] + enabled_clouds = global_user_state.get_enabled_clouds() + if not cloud_type in enabled_clouds: + with ux_utils.print_exception_no_traceback(): + raise exceptions.CloudDisabledError( + f'{store_type} type is specified: {src}. But ' + f'{cloud_type} access is disabled. To fix: enable ' + f'{cloud_type}.' + ) copy_mounts[dst_path] = src # If the src is not a str path, it is likely a dict. Try to # parse storage object. @@ -299,6 +310,17 @@ def from_yaml(yaml_path: str) -> 'Task': mount_path = storage[0] assert mount_path, \ 'Storage mount path cannot be empty.' + store_type = storage[1]['store'] + if store_type: + cloud_type = storage_lib.STORE_TYPE_TO_CLOUD_TYPE[store_type] + enabled_clouds = global_user_state.get_enabled_clouds() + if not cloud_type in enabled_clouds: + with ux_utils.print_exception_no_traceback(): + raise exceptions.CloudDisabledError( + f'Storage \'store:{store_type}\' specified, but ' + f'{cloud_type} access is disabled. To fix: enable ' + f'{cloud_type}.' + ) try: storage_obj = storage_lib.Storage.from_yaml_config(storage[1]) except exceptions.StorageSourceError as e: From 23806117f7eccca1a7ea88d61e0a7126d10320a2 Mon Sep 17 00:00:00 2001 From: Doyoung Kim Date: Wed, 12 Apr 2023 22:06:52 -0700 Subject: [PATCH 02/34] support get_enabled_clouds_str() --- sky/global_user_state.py | 19 +++++++++++++++++-- sky/task.py | 4 ++-- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/sky/global_user_state.py b/sky/global_user_state.py index 5ea5a9c54d4..683e741910f 100644 --- a/sky/global_user_state.py +++ b/sky/global_user_state.py @@ -659,9 +659,24 @@ def get_enabled_clouds() -> List[clouds.Cloud]: cloud = clouds.CLOUD_REGISTRY.from_str(c) if cloud is not None: enabled_clouds.append(cloud) + return enabled_clouds + + +def get_enabled_clouds_str() -> List[clouds.Cloud]: # Currently, 'clouds' only support cloud types with - # computing instances. Hence, the following is a - # temporary solution to support R2 + # computing instances. The following is to temporarily + # support R2 for get_enabled_clouds + rows = _DB.cursor.execute('SELECT value FROM config WHERE key = ?', + (_ENABLED_CLOUDS_KEY,)) + ret = [] + for (value,) in rows: + ret = json.loads(value) + break + enabled_clouds: List[str] = [] + for c in ret: + cloud = clouds.CLOUD_REGISTRY.from_str(c) + if cloud is not None: + enabled_clouds.append(str(cloud)) if os.path.exists( os.path.expanduser(cloudflare.ACCOUNT_ID_PATH)): enabled_clouds.append('Cloudflare') diff --git a/sky/task.py b/sky/task.py index b1a1d7bb88f..4269dda32d7 100644 --- a/sky/task.py +++ b/sky/task.py @@ -285,7 +285,7 @@ def from_yaml(yaml_path: str) -> 'Task': if data_utils.is_cloud_store_url(src): store_type = data_utils.get_cloud_store_type(src) cloud_type = storage_lib.STORE_TYPE_TO_CLOUD_TYPE[store_type] - enabled_clouds = global_user_state.get_enabled_clouds() + enabled_clouds = global_user_state.get_enabled_clouds_str() if not cloud_type in enabled_clouds: with ux_utils.print_exception_no_traceback(): raise exceptions.CloudDisabledError( @@ -313,7 +313,7 @@ def from_yaml(yaml_path: str) -> 'Task': store_type = storage[1]['store'] if store_type: cloud_type = storage_lib.STORE_TYPE_TO_CLOUD_TYPE[store_type] - enabled_clouds = global_user_state.get_enabled_clouds() + enabled_clouds = global_user_state.get_enabled_clouds_str() if not cloud_type in enabled_clouds: with ux_utils.print_exception_no_traceback(): raise exceptions.CloudDisabledError( From 6e3cd170aae40259955d3b04fa952654a4dd56ef Mon Sep 17 00:00:00 2001 From: Doyoung Kim Date: Thu, 13 Apr 2023 05:15:00 +0000 Subject: [PATCH 03/34] fix return type --- sky/data/storage.py | 6 +----- sky/global_user_state.py | 5 ++--- sky/task.py | 12 ++++++------ 3 files changed, 9 insertions(+), 14 deletions(-) diff --git a/sky/data/storage.py b/sky/data/storage.py index 3320e286b78..1b93ae28c7d 100644 --- a/sky/data/storage.py +++ b/sky/data/storage.py @@ -41,11 +41,7 @@ # R2 to be an option as preferred store type STORE_ENABLED_CLOUDS = [clouds.AWS(), clouds.GCP()] -STORE_TYPE_TO_CLOUD_TYPE = { - 's3' : 'AWS', - 'gcs' : 'GCP', - 'r2' : 'Cloudflare' -} +STORE_TYPE_TO_CLOUD_TYPE = {'s3': 'AWS', 'gcs': 'GCP', 'r2': 'Cloudflare'} # Maximum number of concurrent rsync upload processes _MAX_CONCURRENT_UPLOADS = 32 diff --git a/sky/global_user_state.py b/sky/global_user_state.py index 683e741910f..eb6cb2b83de 100644 --- a/sky/global_user_state.py +++ b/sky/global_user_state.py @@ -662,7 +662,7 @@ def get_enabled_clouds() -> List[clouds.Cloud]: return enabled_clouds -def get_enabled_clouds_str() -> List[clouds.Cloud]: +def get_enabled_clouds_str() -> List[str]: # Currently, 'clouds' only support cloud types with # computing instances. The following is to temporarily # support R2 for get_enabled_clouds @@ -677,8 +677,7 @@ def get_enabled_clouds_str() -> List[clouds.Cloud]: cloud = clouds.CLOUD_REGISTRY.from_str(c) if cloud is not None: enabled_clouds.append(str(cloud)) - if os.path.exists( - os.path.expanduser(cloudflare.ACCOUNT_ID_PATH)): + if os.path.exists(os.path.expanduser(cloudflare.ACCOUNT_ID_PATH)): enabled_clouds.append('Cloudflare') return enabled_clouds diff --git a/sky/task.py b/sky/task.py index 4269dda32d7..06148f8ffeb 100644 --- a/sky/task.py +++ b/sky/task.py @@ -284,15 +284,16 @@ def from_yaml(yaml_path: str) -> 'Task': if isinstance(src, str): if data_utils.is_cloud_store_url(src): store_type = data_utils.get_cloud_store_type(src) - cloud_type = storage_lib.STORE_TYPE_TO_CLOUD_TYPE[store_type] - enabled_clouds = global_user_state.get_enabled_clouds_str() + cloud_type = storage_lib.STORE_TYPE_TO_CLOUD_TYPE[ + store_type] + enabled_clouds = global_user_state.get_enabled_clouds_str( + ) if not cloud_type in enabled_clouds: with ux_utils.print_exception_no_traceback(): raise exceptions.CloudDisabledError( f'{store_type} type is specified: {src}. But ' f'{cloud_type} access is disabled. To fix: enable ' - f'{cloud_type}.' - ) + f'{cloud_type}.') copy_mounts[dst_path] = src # If the src is not a str path, it is likely a dict. Try to # parse storage object. @@ -319,8 +320,7 @@ def from_yaml(yaml_path: str) -> 'Task': raise exceptions.CloudDisabledError( f'Storage \'store:{store_type}\' specified, but ' f'{cloud_type} access is disabled. To fix: enable ' - f'{cloud_type}.' - ) + f'{cloud_type}.') try: storage_obj = storage_lib.Storage.from_yaml_config(storage[1]) except exceptions.StorageSourceError as e: From 1b2ae8247bd0f38ebe9822d495d909c1ccf4c34f Mon Sep 17 00:00:00 2001 From: Doyoung Kim Date: Thu, 13 Apr 2023 05:16:27 +0000 Subject: [PATCH 04/34] adding new line eof --- sky/exceptions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sky/exceptions.py b/sky/exceptions.py index 6075f5134a3..8ea4fd2dcfe 100644 --- a/sky/exceptions.py +++ b/sky/exceptions.py @@ -213,4 +213,4 @@ class NoCloudAccessError(Exception): class CloudDisabledError(Exception): """Raised when attempted cloud is disabled""" - pass \ No newline at end of file + pass From 3aa6a35afe60cb732b56c1cbeab11ec014d5ef26 Mon Sep 17 00:00:00 2001 From: Doyoung Kim Date: Wed, 12 Apr 2023 22:24:45 -0700 Subject: [PATCH 05/34] fix error messages --- sky/task.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/sky/task.py b/sky/task.py index 06148f8ffeb..1d1be2cc072 100644 --- a/sky/task.py +++ b/sky/task.py @@ -291,9 +291,9 @@ def from_yaml(yaml_path: str) -> 'Task': if not cloud_type in enabled_clouds: with ux_utils.print_exception_no_traceback(): raise exceptions.CloudDisabledError( - f'{store_type} type is specified: {src}. But ' - f'{cloud_type} access is disabled. To fix: enable ' - f'{cloud_type}.') + f'\'{store_type}\' type is specified: \'{src}\'. But ' + f'\'{cloud_type}\' access is disabled. Enable ' + f'\'{cloud_type}\' to fix.') copy_mounts[dst_path] = src # If the src is not a str path, it is likely a dict. Try to # parse storage object. @@ -319,8 +319,8 @@ def from_yaml(yaml_path: str) -> 'Task': with ux_utils.print_exception_no_traceback(): raise exceptions.CloudDisabledError( f'Storage \'store:{store_type}\' specified, but ' - f'{cloud_type} access is disabled. To fix: enable ' - f'{cloud_type}.') + f'\'{cloud_type}\' access is disabled. Enable ' + f'\'{cloud_type}\' to fix.') try: storage_obj = storage_lib.Storage.from_yaml_config(storage[1]) except exceptions.StorageSourceError as e: From 3b905da3808a9132808d26ad470e648613fab92e Mon Sep 17 00:00:00 2001 From: Doyoung Kim Date: Fri, 14 Apr 2023 01:01:14 -0700 Subject: [PATCH 06/34] update for non-cloud-storage --- sky/task.py | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/sky/task.py b/sky/task.py index 1d1be2cc072..20d5b9462b0 100644 --- a/sky/task.py +++ b/sky/task.py @@ -282,18 +282,6 @@ def from_yaml(yaml_path: str) -> 'Task': for dst_path, src in file_mounts.items(): # Check if it is str path if isinstance(src, str): - if data_utils.is_cloud_store_url(src): - store_type = data_utils.get_cloud_store_type(src) - cloud_type = storage_lib.STORE_TYPE_TO_CLOUD_TYPE[ - store_type] - enabled_clouds = global_user_state.get_enabled_clouds_str( - ) - if not cloud_type in enabled_clouds: - with ux_utils.print_exception_no_traceback(): - raise exceptions.CloudDisabledError( - f'\'{store_type}\' type is specified: \'{src}\'. But ' - f'\'{cloud_type}\' access is disabled. Enable ' - f'\'{cloud_type}\' to fix.') copy_mounts[dst_path] = src # If the src is not a str path, it is likely a dict. Try to # parse storage object. From 845ab23e2ae51062cdb59288ce3eb032ce14ebfc Mon Sep 17 00:00:00 2001 From: Doyoung Kim Date: Sat, 15 Apr 2023 06:12:36 +0000 Subject: [PATCH 07/34] remove unused --- sky/data/data_utils.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/sky/data/data_utils.py b/sky/data/data_utils.py index c51fc93af38..b1048c3ec83 100644 --- a/sky/data/data_utils.py +++ b/sky/data/data_utils.py @@ -111,14 +111,6 @@ def is_cloud_store_url(url): return result.netloc -def get_cloud_store_type(url): - result = urllib.parse.urlsplit(url) - store_type = result.scheme - if store_type == 'gs': - store_type = 'gcs' - return store_type - - def _group_files_by_dir( source_list: List[str]) -> Tuple[Dict[str, List[str]], List[str]]: """Groups a list of paths based on their directory From 3780f6f6f674c5366861bf31e0df64830eb3c746 Mon Sep 17 00:00:00 2001 From: Doyoung Kim Date: Fri, 14 Apr 2023 23:31:05 -0700 Subject: [PATCH 08/34] update check.py, cloudflare.py --- sky/adaptors/cloudflare.py | 8 ++++++++ sky/check.py | 10 +++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/sky/adaptors/cloudflare.py b/sky/adaptors/cloudflare.py index 145665d701b..60e1bb0f89b 100644 --- a/sky/adaptors/cloudflare.py +++ b/sky/adaptors/cloudflare.py @@ -119,3 +119,11 @@ def create_endpoint(): endpoint = 'https://' + accountid + '.r2.cloudflarestorage.com' return endpoint + +def r2_is_enabled(): + """Checks if Cloudflare R2 is enabled""" + + accountid_path = os.path.expanduser(ACCOUNT_ID_PATH) + if os.path.exists(accountid_path): + return True + return False \ No newline at end of file diff --git a/sky/check.py b/sky/check.py index 1a359fa7525..82850bffaf8 100644 --- a/sky/check.py +++ b/sky/check.py @@ -5,7 +5,7 @@ from sky import clouds from sky import global_user_state - +from sky.adaptors import cloudflare def check(quiet: bool = False) -> None: echo = (lambda *_args, **_kwargs: None) if quiet else click.echo @@ -61,4 +61,12 @@ def get_cloud_credential_file_mounts() -> Dict[str, str]: for cloud in enabled_clouds: cloud_file_mounts = cloud.get_credential_file_mounts() file_mounts.update(cloud_file_mounts) + # Currently, get_enabled_clouds() does not support r2 + # as only clouds with computing instances are supported + # by 'clouds' + if cloudflare.r2_is_enabled(): + if not '~/.aws/credentials' in file_mounts: + file_mounts.update({'~/.aws/credentials':'~/.aws/credentials'}) + accountIDPath = cloudflare.ACCOUNT_ID_PATH + file_mounts.update({accountIDPath:accountIDPath}) return file_mounts From 8f06b456145cc8f3f297160a6e9a97049bcc2e52 Mon Sep 17 00:00:00 2001 From: Doyoung Kim Date: Sat, 15 Apr 2023 01:15:58 -0700 Subject: [PATCH 09/34] format --- sky/adaptors/cloudflare.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sky/adaptors/cloudflare.py b/sky/adaptors/cloudflare.py index 60e1bb0f89b..e340b8decb6 100644 --- a/sky/adaptors/cloudflare.py +++ b/sky/adaptors/cloudflare.py @@ -126,4 +126,4 @@ def r2_is_enabled(): accountid_path = os.path.expanduser(ACCOUNT_ID_PATH) if os.path.exists(accountid_path): return True - return False \ No newline at end of file + return False From f9039b1b4df22b091027b6de5396d8509131cc2d Mon Sep 17 00:00:00 2001 From: Doyoung Kim Date: Sat, 15 Apr 2023 01:31:44 -0700 Subject: [PATCH 10/34] format --- sky/check.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sky/check.py b/sky/check.py index 82850bffaf8..b0e2103ec09 100644 --- a/sky/check.py +++ b/sky/check.py @@ -66,7 +66,7 @@ def get_cloud_credential_file_mounts() -> Dict[str, str]: # by 'clouds' if cloudflare.r2_is_enabled(): if not '~/.aws/credentials' in file_mounts: - file_mounts.update({'~/.aws/credentials':'~/.aws/credentials'}) + file_mounts.update({'~/.aws/credentials': '~/.aws/credentials'}) accountIDPath = cloudflare.ACCOUNT_ID_PATH - file_mounts.update({accountIDPath:accountIDPath}) + file_mounts.update({accountIDPath: accountIDPath}) return file_mounts From bbf50934bd9a3bd5f946575afb27c910f27c2ed2 Mon Sep 17 00:00:00 2001 From: Doyoung Kim Date: Sat, 15 Apr 2023 01:38:35 -0700 Subject: [PATCH 11/34] format --- sky/adaptors/cloudflare.py | 3 ++- sky/check.py | 7 ++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/sky/adaptors/cloudflare.py b/sky/adaptors/cloudflare.py index e340b8decb6..f899fb9542c 100644 --- a/sky/adaptors/cloudflare.py +++ b/sky/adaptors/cloudflare.py @@ -120,9 +120,10 @@ def create_endpoint(): return endpoint + def r2_is_enabled(): """Checks if Cloudflare R2 is enabled""" - + accountid_path = os.path.expanduser(ACCOUNT_ID_PATH) if os.path.exists(accountid_path): return True diff --git a/sky/check.py b/sky/check.py index b0e2103ec09..813ec1b98a8 100644 --- a/sky/check.py +++ b/sky/check.py @@ -7,6 +7,7 @@ from sky import global_user_state from sky.adaptors import cloudflare + def check(quiet: bool = False) -> None: echo = (lambda *_args, **_kwargs: None) if quiet else click.echo echo('Checking credentials to enable clouds for SkyPilot.') @@ -65,8 +66,8 @@ def get_cloud_credential_file_mounts() -> Dict[str, str]: # as only clouds with computing instances are supported # by 'clouds' if cloudflare.r2_is_enabled(): - if not '~/.aws/credentials' in file_mounts: + if '~/.aws/credentials' not in file_mounts: file_mounts.update({'~/.aws/credentials': '~/.aws/credentials'}) - accountIDPath = cloudflare.ACCOUNT_ID_PATH - file_mounts.update({accountIDPath: accountIDPath}) + accnt_id_path = cloudflare.ACCOUNT_ID_PATH + file_mounts.update({accnt_id_path: accnt_id_path}) return file_mounts From 2a82a09ce2330cd2cb193f07b8fc4152e69ef061 Mon Sep 17 00:00:00 2001 From: Doyoung Kim Date: Sat, 15 Apr 2023 15:15:26 -0700 Subject: [PATCH 12/34] update fix --- sky/adaptors/cloudflare.py | 21 +++++++++++++++++---- sky/check.py | 11 +++++------ 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/sky/adaptors/cloudflare.py b/sky/adaptors/cloudflare.py index f899fb9542c..db876b6c733 100644 --- a/sky/adaptors/cloudflare.py +++ b/sky/adaptors/cloudflare.py @@ -5,6 +5,7 @@ import functools import threading import os +from typing import Dict boto3 = None botocore = None @@ -121,10 +122,22 @@ def create_endpoint(): return endpoint -def r2_is_enabled(): +def r2_is_enabled() -> bool: """Checks if Cloudflare R2 is enabled""" accountid_path = os.path.expanduser(ACCOUNT_ID_PATH) - if os.path.exists(accountid_path): - return True - return False + return os.path.exists(accountid_path) + +def get_credential_file_mounts(file_mounts: Dict[str, str]) -> Dict[str, str]: + """Checks if aws credential file is set and update if not + Updates file containing account ID information + + Args: + file_mounts: stores path to credential files of clouds + """ + + r2_credential_mounts = {} + if '~/.aws/credentials' not in file_mounts: + r2_credential_mounts.update({'~/.aws/credentials': '~/.aws/credentials'}) + r2_credential_mounts.update({ACCOUNT_ID_PATH: ACCOUNT_ID_PATH}) + return r2_credential_mounts \ No newline at end of file diff --git a/sky/check.py b/sky/check.py index 813ec1b98a8..672cb1d5f28 100644 --- a/sky/check.py +++ b/sky/check.py @@ -63,11 +63,10 @@ def get_cloud_credential_file_mounts() -> Dict[str, str]: cloud_file_mounts = cloud.get_credential_file_mounts() file_mounts.update(cloud_file_mounts) # Currently, get_enabled_clouds() does not support r2 - # as only clouds with computing instances are supported - # by 'clouds' + # as only clouds with computing instances are marked + # as enabled by skypilot. This will be removed when + # cloudflare/r2 is added as a 'cloud'. if cloudflare.r2_is_enabled(): - if '~/.aws/credentials' not in file_mounts: - file_mounts.update({'~/.aws/credentials': '~/.aws/credentials'}) - accnt_id_path = cloudflare.ACCOUNT_ID_PATH - file_mounts.update({accnt_id_path: accnt_id_path}) + r2_credential_mounts = cloudflare.get_credential_file_mounts(file_mounts) + file_mounts.update(r2_credential_mounts) return file_mounts From 754008547f90dfdf0d12a2ca92ee176e9fca29a1 Mon Sep 17 00:00:00 2001 From: Doyoung Kim Date: Wed, 12 Apr 2023 21:48:57 -0700 Subject: [PATCH 13/34] ux_optimization_disabled_clouds --- sky/data/data_utils.py | 8 ++++++++ sky/data/storage.py | 6 ++++++ sky/exceptions.py | 5 +++++ sky/global_user_state.py | 7 +++++++ sky/task.py | 22 ++++++++++++++++++++++ 5 files changed, 48 insertions(+) diff --git a/sky/data/data_utils.py b/sky/data/data_utils.py index b1048c3ec83..c51fc93af38 100644 --- a/sky/data/data_utils.py +++ b/sky/data/data_utils.py @@ -111,6 +111,14 @@ def is_cloud_store_url(url): return result.netloc +def get_cloud_store_type(url): + result = urllib.parse.urlsplit(url) + store_type = result.scheme + if store_type == 'gs': + store_type = 'gcs' + return store_type + + def _group_files_by_dir( source_list: List[str]) -> Tuple[Dict[str, List[str]], List[str]]: """Groups a list of paths based on their directory diff --git a/sky/data/storage.py b/sky/data/storage.py index a2355bac57a..5e404a628d5 100644 --- a/sky/data/storage.py +++ b/sky/data/storage.py @@ -42,6 +42,12 @@ # R2 to be an option as preferred store type STORE_ENABLED_CLOUDS = [clouds.AWS(), clouds.GCP()] +STORE_TYPE_TO_CLOUD_TYPE = { + 's3' : 'AWS', + 'gcs' : 'GCP', + 'r2' : 'Cloudflare' +} + # Maximum number of concurrent rsync upload processes _MAX_CONCURRENT_UPLOADS = 32 diff --git a/sky/exceptions.py b/sky/exceptions.py index 0211cb81efa..6075f5134a3 100644 --- a/sky/exceptions.py +++ b/sky/exceptions.py @@ -209,3 +209,8 @@ class ClusterOwnerIdentityMismatchError(Exception): class NoCloudAccessError(Exception): """Raised when all clouds are disabled.""" pass + + +class CloudDisabledError(Exception): + """Raised when attempted cloud is disabled""" + pass \ No newline at end of file diff --git a/sky/global_user_state.py b/sky/global_user_state.py index 4bd6ff67912..8927f9c0e6f 100644 --- a/sky/global_user_state.py +++ b/sky/global_user_state.py @@ -22,6 +22,7 @@ from sky import clouds from sky.utils import db_utils from sky.utils import common_utils +from sky.adaptors import cloudflare if typing.TYPE_CHECKING: from sky import backends @@ -660,6 +661,12 @@ def get_enabled_clouds() -> List[clouds.Cloud]: cloud = clouds.CLOUD_REGISTRY.from_str(c) if cloud is not None: enabled_clouds.append(cloud) + # Currently, 'clouds' only support cloud types with + # computing instances. Hence, the following is a + # temporary solution to support R2 + if os.path.exists( + os.path.expanduser(cloudflare.ACCOUNT_ID_PATH)): + enabled_clouds.append('Cloudflare') return enabled_clouds diff --git a/sky/task.py b/sky/task.py index 58833aa841e..44e8ecdeb9d 100644 --- a/sky/task.py +++ b/sky/task.py @@ -282,6 +282,17 @@ def from_yaml(yaml_path: str) -> 'Task': for dst_path, src in file_mounts.items(): # Check if it is str path if isinstance(src, str): + if data_utils.is_cloud_store_url(src): + store_type = data_utils.get_cloud_store_type(src) + cloud_type = storage_lib.STORE_TYPE_TO_CLOUD_TYPE[store_type] + enabled_clouds = global_user_state.get_enabled_clouds() + if not cloud_type in enabled_clouds: + with ux_utils.print_exception_no_traceback(): + raise exceptions.CloudDisabledError( + f'{store_type} type is specified: {src}. But ' + f'{cloud_type} access is disabled. To fix: enable ' + f'{cloud_type}.' + ) copy_mounts[dst_path] = src # If the src is not a str path, it is likely a dict. Try to # parse storage object. @@ -299,6 +310,17 @@ def from_yaml(yaml_path: str) -> 'Task': mount_path = storage[0] assert mount_path, \ 'Storage mount path cannot be empty.' + store_type = storage[1]['store'] + if store_type: + cloud_type = storage_lib.STORE_TYPE_TO_CLOUD_TYPE[store_type] + enabled_clouds = global_user_state.get_enabled_clouds() + if not cloud_type in enabled_clouds: + with ux_utils.print_exception_no_traceback(): + raise exceptions.CloudDisabledError( + f'Storage \'store:{store_type}\' specified, but ' + f'{cloud_type} access is disabled. To fix: enable ' + f'{cloud_type}.' + ) try: storage_obj = storage_lib.Storage.from_yaml_config(storage[1]) except exceptions.StorageSourceError as e: From 66ad186b07b3bf3390442b46fa4277594c68499f Mon Sep 17 00:00:00 2001 From: Doyoung Kim Date: Wed, 12 Apr 2023 22:06:52 -0700 Subject: [PATCH 14/34] support get_enabled_clouds_str() --- sky/global_user_state.py | 19 +++++++++++++++++-- sky/task.py | 4 ++-- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/sky/global_user_state.py b/sky/global_user_state.py index 8927f9c0e6f..b25cbf028a0 100644 --- a/sky/global_user_state.py +++ b/sky/global_user_state.py @@ -661,9 +661,24 @@ def get_enabled_clouds() -> List[clouds.Cloud]: cloud = clouds.CLOUD_REGISTRY.from_str(c) if cloud is not None: enabled_clouds.append(cloud) + return enabled_clouds + + +def get_enabled_clouds_str() -> List[clouds.Cloud]: # Currently, 'clouds' only support cloud types with - # computing instances. Hence, the following is a - # temporary solution to support R2 + # computing instances. The following is to temporarily + # support R2 for get_enabled_clouds + rows = _DB.cursor.execute('SELECT value FROM config WHERE key = ?', + (_ENABLED_CLOUDS_KEY,)) + ret = [] + for (value,) in rows: + ret = json.loads(value) + break + enabled_clouds: List[str] = [] + for c in ret: + cloud = clouds.CLOUD_REGISTRY.from_str(c) + if cloud is not None: + enabled_clouds.append(str(cloud)) if os.path.exists( os.path.expanduser(cloudflare.ACCOUNT_ID_PATH)): enabled_clouds.append('Cloudflare') diff --git a/sky/task.py b/sky/task.py index 44e8ecdeb9d..f203edd7c67 100644 --- a/sky/task.py +++ b/sky/task.py @@ -285,7 +285,7 @@ def from_yaml(yaml_path: str) -> 'Task': if data_utils.is_cloud_store_url(src): store_type = data_utils.get_cloud_store_type(src) cloud_type = storage_lib.STORE_TYPE_TO_CLOUD_TYPE[store_type] - enabled_clouds = global_user_state.get_enabled_clouds() + enabled_clouds = global_user_state.get_enabled_clouds_str() if not cloud_type in enabled_clouds: with ux_utils.print_exception_no_traceback(): raise exceptions.CloudDisabledError( @@ -313,7 +313,7 @@ def from_yaml(yaml_path: str) -> 'Task': store_type = storage[1]['store'] if store_type: cloud_type = storage_lib.STORE_TYPE_TO_CLOUD_TYPE[store_type] - enabled_clouds = global_user_state.get_enabled_clouds() + enabled_clouds = global_user_state.get_enabled_clouds_str() if not cloud_type in enabled_clouds: with ux_utils.print_exception_no_traceback(): raise exceptions.CloudDisabledError( From da89eb85d0ac05b1b11e0fc816c910eab722e631 Mon Sep 17 00:00:00 2001 From: Doyoung Kim Date: Thu, 13 Apr 2023 05:15:00 +0000 Subject: [PATCH 15/34] fix return type --- sky/data/storage.py | 6 +----- sky/global_user_state.py | 5 ++--- sky/task.py | 12 ++++++------ 3 files changed, 9 insertions(+), 14 deletions(-) diff --git a/sky/data/storage.py b/sky/data/storage.py index 5e404a628d5..ecd22bac798 100644 --- a/sky/data/storage.py +++ b/sky/data/storage.py @@ -42,11 +42,7 @@ # R2 to be an option as preferred store type STORE_ENABLED_CLOUDS = [clouds.AWS(), clouds.GCP()] -STORE_TYPE_TO_CLOUD_TYPE = { - 's3' : 'AWS', - 'gcs' : 'GCP', - 'r2' : 'Cloudflare' -} +STORE_TYPE_TO_CLOUD_TYPE = {'s3': 'AWS', 'gcs': 'GCP', 'r2': 'Cloudflare'} # Maximum number of concurrent rsync upload processes _MAX_CONCURRENT_UPLOADS = 32 diff --git a/sky/global_user_state.py b/sky/global_user_state.py index b25cbf028a0..1c45685d920 100644 --- a/sky/global_user_state.py +++ b/sky/global_user_state.py @@ -664,7 +664,7 @@ def get_enabled_clouds() -> List[clouds.Cloud]: return enabled_clouds -def get_enabled_clouds_str() -> List[clouds.Cloud]: +def get_enabled_clouds_str() -> List[str]: # Currently, 'clouds' only support cloud types with # computing instances. The following is to temporarily # support R2 for get_enabled_clouds @@ -679,8 +679,7 @@ def get_enabled_clouds_str() -> List[clouds.Cloud]: cloud = clouds.CLOUD_REGISTRY.from_str(c) if cloud is not None: enabled_clouds.append(str(cloud)) - if os.path.exists( - os.path.expanduser(cloudflare.ACCOUNT_ID_PATH)): + if os.path.exists(os.path.expanduser(cloudflare.ACCOUNT_ID_PATH)): enabled_clouds.append('Cloudflare') return enabled_clouds diff --git a/sky/task.py b/sky/task.py index f203edd7c67..877776331f5 100644 --- a/sky/task.py +++ b/sky/task.py @@ -284,15 +284,16 @@ def from_yaml(yaml_path: str) -> 'Task': if isinstance(src, str): if data_utils.is_cloud_store_url(src): store_type = data_utils.get_cloud_store_type(src) - cloud_type = storage_lib.STORE_TYPE_TO_CLOUD_TYPE[store_type] - enabled_clouds = global_user_state.get_enabled_clouds_str() + cloud_type = storage_lib.STORE_TYPE_TO_CLOUD_TYPE[ + store_type] + enabled_clouds = global_user_state.get_enabled_clouds_str( + ) if not cloud_type in enabled_clouds: with ux_utils.print_exception_no_traceback(): raise exceptions.CloudDisabledError( f'{store_type} type is specified: {src}. But ' f'{cloud_type} access is disabled. To fix: enable ' - f'{cloud_type}.' - ) + f'{cloud_type}.') copy_mounts[dst_path] = src # If the src is not a str path, it is likely a dict. Try to # parse storage object. @@ -319,8 +320,7 @@ def from_yaml(yaml_path: str) -> 'Task': raise exceptions.CloudDisabledError( f'Storage \'store:{store_type}\' specified, but ' f'{cloud_type} access is disabled. To fix: enable ' - f'{cloud_type}.' - ) + f'{cloud_type}.') try: storage_obj = storage_lib.Storage.from_yaml_config(storage[1]) except exceptions.StorageSourceError as e: From af9fdb3481c392ef6e413bb45a65fe5a0619d59e Mon Sep 17 00:00:00 2001 From: Doyoung Kim Date: Thu, 13 Apr 2023 05:16:27 +0000 Subject: [PATCH 16/34] adding new line eof --- sky/exceptions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sky/exceptions.py b/sky/exceptions.py index 6075f5134a3..8ea4fd2dcfe 100644 --- a/sky/exceptions.py +++ b/sky/exceptions.py @@ -213,4 +213,4 @@ class NoCloudAccessError(Exception): class CloudDisabledError(Exception): """Raised when attempted cloud is disabled""" - pass \ No newline at end of file + pass From 67375965ed040f7051d2552efff3651d87cb39ed Mon Sep 17 00:00:00 2001 From: Doyoung Kim Date: Wed, 12 Apr 2023 22:24:45 -0700 Subject: [PATCH 17/34] fix error messages --- sky/task.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/sky/task.py b/sky/task.py index 877776331f5..9a53a5c10b8 100644 --- a/sky/task.py +++ b/sky/task.py @@ -291,9 +291,9 @@ def from_yaml(yaml_path: str) -> 'Task': if not cloud_type in enabled_clouds: with ux_utils.print_exception_no_traceback(): raise exceptions.CloudDisabledError( - f'{store_type} type is specified: {src}. But ' - f'{cloud_type} access is disabled. To fix: enable ' - f'{cloud_type}.') + f'\'{store_type}\' type is specified: \'{src}\'. But ' + f'\'{cloud_type}\' access is disabled. Enable ' + f'\'{cloud_type}\' to fix.') copy_mounts[dst_path] = src # If the src is not a str path, it is likely a dict. Try to # parse storage object. @@ -319,8 +319,8 @@ def from_yaml(yaml_path: str) -> 'Task': with ux_utils.print_exception_no_traceback(): raise exceptions.CloudDisabledError( f'Storage \'store:{store_type}\' specified, but ' - f'{cloud_type} access is disabled. To fix: enable ' - f'{cloud_type}.') + f'\'{cloud_type}\' access is disabled. Enable ' + f'\'{cloud_type}\' to fix.') try: storage_obj = storage_lib.Storage.from_yaml_config(storage[1]) except exceptions.StorageSourceError as e: From 629980411c3f03055d282fae8b056bbb400e42f6 Mon Sep 17 00:00:00 2001 From: Doyoung Kim Date: Sat, 15 Apr 2023 06:12:36 +0000 Subject: [PATCH 18/34] remove unused --- sky/data/data_utils.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/sky/data/data_utils.py b/sky/data/data_utils.py index c51fc93af38..b1048c3ec83 100644 --- a/sky/data/data_utils.py +++ b/sky/data/data_utils.py @@ -111,14 +111,6 @@ def is_cloud_store_url(url): return result.netloc -def get_cloud_store_type(url): - result = urllib.parse.urlsplit(url) - store_type = result.scheme - if store_type == 'gs': - store_type = 'gcs' - return store_type - - def _group_files_by_dir( source_list: List[str]) -> Tuple[Dict[str, List[str]], List[str]]: """Groups a list of paths based on their directory From d19e3fdf2922c20c57a3b0076375afa21c73191a Mon Sep 17 00:00:00 2001 From: Doyoung Kim Date: Fri, 14 Apr 2023 01:01:14 -0700 Subject: [PATCH 19/34] update for non-cloud-storage --- sky/task.py | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/sky/task.py b/sky/task.py index 9a53a5c10b8..830a2be45b1 100644 --- a/sky/task.py +++ b/sky/task.py @@ -282,18 +282,6 @@ def from_yaml(yaml_path: str) -> 'Task': for dst_path, src in file_mounts.items(): # Check if it is str path if isinstance(src, str): - if data_utils.is_cloud_store_url(src): - store_type = data_utils.get_cloud_store_type(src) - cloud_type = storage_lib.STORE_TYPE_TO_CLOUD_TYPE[ - store_type] - enabled_clouds = global_user_state.get_enabled_clouds_str( - ) - if not cloud_type in enabled_clouds: - with ux_utils.print_exception_no_traceback(): - raise exceptions.CloudDisabledError( - f'\'{store_type}\' type is specified: \'{src}\'. But ' - f'\'{cloud_type}\' access is disabled. Enable ' - f'\'{cloud_type}\' to fix.') copy_mounts[dst_path] = src # If the src is not a str path, it is likely a dict. Try to # parse storage object. From 6ee395282ce0c5e5203e5bf4cdb7d7fd9097802d Mon Sep 17 00:00:00 2001 From: Doyoung Kim Date: Sat, 15 Apr 2023 16:22:44 -0700 Subject: [PATCH 20/34] rebasing --- sky/global_user_state.py | 20 -------------------- sky/task.py | 8 ++++++-- 2 files changed, 6 insertions(+), 22 deletions(-) diff --git a/sky/global_user_state.py b/sky/global_user_state.py index 1c45685d920..ea828a919e6 100644 --- a/sky/global_user_state.py +++ b/sky/global_user_state.py @@ -664,26 +664,6 @@ def get_enabled_clouds() -> List[clouds.Cloud]: return enabled_clouds -def get_enabled_clouds_str() -> List[str]: - # Currently, 'clouds' only support cloud types with - # computing instances. The following is to temporarily - # support R2 for get_enabled_clouds - rows = _DB.cursor.execute('SELECT value FROM config WHERE key = ?', - (_ENABLED_CLOUDS_KEY,)) - ret = [] - for (value,) in rows: - ret = json.loads(value) - break - enabled_clouds: List[str] = [] - for c in ret: - cloud = clouds.CLOUD_REGISTRY.from_str(c) - if cloud is not None: - enabled_clouds.append(str(cloud)) - if os.path.exists(os.path.expanduser(cloudflare.ACCOUNT_ID_PATH)): - enabled_clouds.append('Cloudflare') - return enabled_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 830a2be45b1..87252aca64f 100644 --- a/sky/task.py +++ b/sky/task.py @@ -17,6 +17,7 @@ from sky.skylet import constants from sky.utils import schemas from sky.utils import ux_utils +from sky.adaptors import cloudflare if typing.TYPE_CHECKING: from sky import resources as resources_lib @@ -295,6 +296,10 @@ def from_yaml(yaml_path: str) -> 'Task': task_storage_mounts: Dict[str, storage_lib.Storage] = {} all_storages = fm_storages + enabled_clouds = global_user_state.get_enabled_clouds() + enabled_clouds = [str(cloud) for cloud in enabled_clouds] + if cloudflare.r2_is_enabled: + enabled_clouds.append('r2') for storage in all_storages: mount_path = storage[0] assert mount_path, \ @@ -302,8 +307,7 @@ def from_yaml(yaml_path: str) -> 'Task': store_type = storage[1]['store'] if store_type: cloud_type = storage_lib.STORE_TYPE_TO_CLOUD_TYPE[store_type] - enabled_clouds = global_user_state.get_enabled_clouds_str() - if not cloud_type in enabled_clouds: + if cloud_type not in enabled_clouds: with ux_utils.print_exception_no_traceback(): raise exceptions.CloudDisabledError( f'Storage \'store:{store_type}\' specified, but ' From ec04950246b75d2c0387bb8418f6949e70cf4592 Mon Sep 17 00:00:00 2001 From: Doyoung Kim Date: Sat, 15 Apr 2023 23:33:37 +0000 Subject: [PATCH 21/34] format --- sky/adaptors/cloudflare.py | 8 +++++--- sky/check.py | 3 ++- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/sky/adaptors/cloudflare.py b/sky/adaptors/cloudflare.py index db876b6c733..49e00e66fbc 100644 --- a/sky/adaptors/cloudflare.py +++ b/sky/adaptors/cloudflare.py @@ -128,6 +128,7 @@ def r2_is_enabled() -> bool: accountid_path = os.path.expanduser(ACCOUNT_ID_PATH) return os.path.exists(accountid_path) + def get_credential_file_mounts(file_mounts: Dict[str, str]) -> Dict[str, str]: """Checks if aws credential file is set and update if not Updates file containing account ID information @@ -135,9 +136,10 @@ def get_credential_file_mounts(file_mounts: Dict[str, str]) -> Dict[str, str]: Args: file_mounts: stores path to credential files of clouds """ - + r2_credential_mounts = {} if '~/.aws/credentials' not in file_mounts: - r2_credential_mounts.update({'~/.aws/credentials': '~/.aws/credentials'}) + r2_credential_mounts.update( + {'~/.aws/credentials': '~/.aws/credentials'}) r2_credential_mounts.update({ACCOUNT_ID_PATH: ACCOUNT_ID_PATH}) - return r2_credential_mounts \ No newline at end of file + return r2_credential_mounts diff --git a/sky/check.py b/sky/check.py index 672cb1d5f28..bdac8ae5c97 100644 --- a/sky/check.py +++ b/sky/check.py @@ -67,6 +67,7 @@ def get_cloud_credential_file_mounts() -> Dict[str, str]: # as enabled by skypilot. This will be removed when # cloudflare/r2 is added as a 'cloud'. if cloudflare.r2_is_enabled(): - r2_credential_mounts = cloudflare.get_credential_file_mounts(file_mounts) + r2_credential_mounts = cloudflare.get_credential_file_mounts( + file_mounts) file_mounts.update(r2_credential_mounts) return file_mounts From 256b6f36b9c3cc2be8ac44a77bf83dd8422b6d1a Mon Sep 17 00:00:00 2001 From: Doyoung Kim Date: Sat, 15 Apr 2023 23:35:18 +0000 Subject: [PATCH 22/34] fix --- sky/task.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sky/task.py b/sky/task.py index 87252aca64f..f7b12b10d2f 100644 --- a/sky/task.py +++ b/sky/task.py @@ -298,7 +298,7 @@ def from_yaml(yaml_path: str) -> 'Task': all_storages = fm_storages enabled_clouds = global_user_state.get_enabled_clouds() enabled_clouds = [str(cloud) for cloud in enabled_clouds] - if cloudflare.r2_is_enabled: + if cloudflare.r2_is_enabled(): enabled_clouds.append('r2') for storage in all_storages: mount_path = storage[0] From cf874da95867f92bf0e9b84a2765ca3512fd334d Mon Sep 17 00:00:00 2001 From: Doyoung Kim Date: Sat, 15 Apr 2023 23:41:22 +0000 Subject: [PATCH 23/34] format --- sky/adaptors/cloudflare.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sky/adaptors/cloudflare.py b/sky/adaptors/cloudflare.py index 49e00e66fbc..6a7da5781b3 100644 --- a/sky/adaptors/cloudflare.py +++ b/sky/adaptors/cloudflare.py @@ -132,7 +132,7 @@ def r2_is_enabled() -> bool: def get_credential_file_mounts(file_mounts: Dict[str, str]) -> Dict[str, str]: """Checks if aws credential file is set and update if not Updates file containing account ID information - + Args: file_mounts: stores path to credential files of clouds """ From bc02cb746dbb5acd78d7dee40beabf658a05d113 Mon Sep 17 00:00:00 2001 From: Doyoung Kim Date: Wed, 19 Apr 2023 23:59:07 -0700 Subject: [PATCH 24/34] fix --- sky/adaptors/cloudflare.py | 56 +++++++++++++++++++++++++++----------- sky/check.py | 9 +++--- sky/task.py | 3 +- 3 files changed, 46 insertions(+), 22 deletions(-) diff --git a/sky/adaptors/cloudflare.py b/sky/adaptors/cloudflare.py index 6a7da5781b3..77c01f7df1a 100644 --- a/sky/adaptors/cloudflare.py +++ b/sky/adaptors/cloudflare.py @@ -5,12 +5,13 @@ import functools import threading import os -from typing import Dict +from typing import Dict, Optional, Tuple boto3 = None botocore = None _session_creation_lock = threading.RLock() ACCOUNT_ID_PATH = '~/.cloudflare/accountid' +AWS_R2_PROFILE_PATH = '~/.aws/credentials' R2_PROFILE_NAME = 'r2' @@ -53,7 +54,6 @@ def session(): @import_package def resource(resource_name: str, **kwargs): """Create a Cloudflare resource. - Args: resource_name: Cloudflare resource name (e.g., 's3'). kwargs: Other options. @@ -79,7 +79,6 @@ def resource(resource_name: str, **kwargs): @functools.lru_cache() def client(service_name: str, region): """Create an CLOUDFLARE client of a certain service. - Args: service_name: CLOUDFLARE service name (e.g., 's3'). kwargs: Other options. @@ -122,24 +121,49 @@ def create_endpoint(): return endpoint -def r2_is_enabled() -> bool: - """Checks if Cloudflare R2 is enabled""" +def check_credentials() -> Tuple[bool, Optional[str]]: + """Checks if the user has access credentials to this cloud.""" + hints = None accountid_path = os.path.expanduser(ACCOUNT_ID_PATH) - return os.path.exists(accountid_path) - - -def get_credential_file_mounts(file_mounts: Dict[str, str]) -> Dict[str, str]: + if not os.path.exists(accountid_path): + hints = 'Account ID from R2 dashboard is not set.' + if not r2_profile_in_aws_cred(): + if hints: + hints += ' And ' + else: + hints = '' + hints += '[r2] profile is not set in ~/.aws/credentials.' + if hints: + hints += ( + '\n Please follow the instructions in:' + '\n https://skypilot.readthedocs.io/en/latest/getting-started/installation.html#cloudflare-r2' + ) + return (False, hints) if hints else (True, hints) + + +def r2_profile_in_aws_cred() -> bool: + """Checks if Cloudflare R2 profile is set in aws credentials""" + + profile_path = os.path.expanduser(AWS_R2_PROFILE_PATH) + r2_profile_exists = False + if os.path.isfile(profile_path): + with open(profile_path, 'r') as file: + for line in file: + if '[r2]' in line: + r2_profile_exists = True + return r2_profile_exists + + +def get_credential_file_mounts() -> Dict[str, str]: """Checks if aws credential file is set and update if not Updates file containing account ID information - Args: file_mounts: stores path to credential files of clouds """ - r2_credential_mounts = {} - if '~/.aws/credentials' not in file_mounts: - r2_credential_mounts.update( - {'~/.aws/credentials': '~/.aws/credentials'}) - r2_credential_mounts.update({ACCOUNT_ID_PATH: ACCOUNT_ID_PATH}) - return r2_credential_mounts + r2_credential_mounts = { + AWS_R2_PROFILE_PATH: AWS_R2_PROFILE_PATH, + ACCOUNT_ID_PATH: ACCOUNT_ID_PATH + } + return r2_credential_mounts \ No newline at end of file diff --git a/sky/check.py b/sky/check.py index bdac8ae5c97..6e6ab8aad91 100644 --- a/sky/check.py +++ b/sky/check.py @@ -53,7 +53,6 @@ def check(quiet: bool = False) -> None: def get_cloud_credential_file_mounts() -> Dict[str, str]: """Returns the files necessary to access all enabled clouds. - Returns a dictionary that will be added to a task's file mounts and a list of patterns that will be excluded (used as rsync_exclude). """ @@ -66,8 +65,8 @@ def get_cloud_credential_file_mounts() -> Dict[str, str]: # as only clouds with computing instances are marked # as enabled by skypilot. This will be removed when # cloudflare/r2 is added as a 'cloud'. - if cloudflare.r2_is_enabled(): - r2_credential_mounts = cloudflare.get_credential_file_mounts( - file_mounts) + r2_is_enabled, _ = cloud = cloudflare.check_credentials() + if r2_is_enabled: + r2_credential_mounts = cloudflare.get_credential_file_mounts() file_mounts.update(r2_credential_mounts) - return file_mounts + return file_mounts \ No newline at end of file diff --git a/sky/task.py b/sky/task.py index f7b12b10d2f..0035239ccf2 100644 --- a/sky/task.py +++ b/sky/task.py @@ -298,7 +298,8 @@ def from_yaml(yaml_path: str) -> 'Task': all_storages = fm_storages enabled_clouds = global_user_state.get_enabled_clouds() enabled_clouds = [str(cloud) for cloud in enabled_clouds] - if cloudflare.r2_is_enabled(): + r2_is_enabled, _ = cloudflare.check_credentials() + if r2_is_enabled: enabled_clouds.append('r2') for storage in all_storages: mount_path = storage[0] From 5acdbbe77658d0f5380471192578005465eb9647 Mon Sep 17 00:00:00 2001 From: Doyoung Kim Date: Thu, 20 Apr 2023 00:03:32 -0700 Subject: [PATCH 25/34] format --- sky/adaptors/cloudflare.py | 3 ++- sky/check.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/sky/adaptors/cloudflare.py b/sky/adaptors/cloudflare.py index 77c01f7df1a..859fe104b6c 100644 --- a/sky/adaptors/cloudflare.py +++ b/sky/adaptors/cloudflare.py @@ -166,4 +166,5 @@ def get_credential_file_mounts() -> Dict[str, str]: AWS_R2_PROFILE_PATH: AWS_R2_PROFILE_PATH, ACCOUNT_ID_PATH: ACCOUNT_ID_PATH } - return r2_credential_mounts \ No newline at end of file + return r2_credential_mounts + \ No newline at end of file diff --git a/sky/check.py b/sky/check.py index 6e6ab8aad91..7b29351c89d 100644 --- a/sky/check.py +++ b/sky/check.py @@ -69,4 +69,5 @@ def get_cloud_credential_file_mounts() -> Dict[str, str]: if r2_is_enabled: r2_credential_mounts = cloudflare.get_credential_file_mounts() file_mounts.update(r2_credential_mounts) - return file_mounts \ No newline at end of file + return file_mounts + \ No newline at end of file From 0c2b2b17393b80da2b6aa6de24d03fcff37248ae Mon Sep 17 00:00:00 2001 From: Doyoung Kim Date: Thu, 20 Apr 2023 00:14:50 -0700 Subject: [PATCH 26/34] format --- sky/adaptors/cloudflare.py | 1 - sky/check.py | 1 - 2 files changed, 2 deletions(-) diff --git a/sky/adaptors/cloudflare.py b/sky/adaptors/cloudflare.py index 859fe104b6c..f2a75d1eb7d 100644 --- a/sky/adaptors/cloudflare.py +++ b/sky/adaptors/cloudflare.py @@ -167,4 +167,3 @@ def get_credential_file_mounts() -> Dict[str, str]: ACCOUNT_ID_PATH: ACCOUNT_ID_PATH } return r2_credential_mounts - \ No newline at end of file diff --git a/sky/check.py b/sky/check.py index 7b29351c89d..c8c565f0810 100644 --- a/sky/check.py +++ b/sky/check.py @@ -70,4 +70,3 @@ def get_cloud_credential_file_mounts() -> Dict[str, str]: r2_credential_mounts = cloudflare.get_credential_file_mounts() file_mounts.update(r2_credential_mounts) return file_mounts - \ No newline at end of file From 4f28c4aed732b89a2974763471dc529ff2093c14 Mon Sep 17 00:00:00 2001 From: Doyoung Kim Date: Thu, 4 May 2023 20:46:54 -0700 Subject: [PATCH 27/34] nit --- sky/adaptors/cloudflare.py | 4 +++- sky/check.py | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/sky/adaptors/cloudflare.py b/sky/adaptors/cloudflare.py index c0a9b47121b..1685d0babe9 100644 --- a/sky/adaptors/cloudflare.py +++ b/sky/adaptors/cloudflare.py @@ -54,6 +54,7 @@ def session(): @import_package def resource(resource_name: str, **kwargs): """Create a Cloudflare resource. + Args: resource_name: Cloudflare resource name (e.g., 's3'). kwargs: Other options. @@ -79,6 +80,7 @@ def resource(resource_name: str, **kwargs): @functools.lru_cache() def client(service_name: str, region): """Create an CLOUDFLARE client of a certain service. + Args: service_name: CLOUDFLARE service name (e.g., 's3'). kwargs: Other options. @@ -167,6 +169,7 @@ def r2_profile_in_aws_cred() -> bool: def get_credential_file_mounts() -> Dict[str, str]: """Checks if aws credential file is set and update if not Updates file containing account ID information + Args: file_mounts: stores path to credential files of clouds """ @@ -176,4 +179,3 @@ def get_credential_file_mounts() -> Dict[str, str]: ACCOUNT_ID_PATH: ACCOUNT_ID_PATH } return r2_credential_mounts - \ No newline at end of file diff --git a/sky/check.py b/sky/check.py index c8c565f0810..b4574e93bfc 100644 --- a/sky/check.py +++ b/sky/check.py @@ -53,6 +53,7 @@ def check(quiet: bool = False) -> None: def get_cloud_credential_file_mounts() -> Dict[str, str]: """Returns the files necessary to access all enabled clouds. + Returns a dictionary that will be added to a task's file mounts and a list of patterns that will be excluded (used as rsync_exclude). """ From 7f57e6da11cc6a3f921de2af578411a94bfee288 Mon Sep 17 00:00:00 2001 From: Doyoung Kim Date: Thu, 4 May 2023 20:55:57 -0700 Subject: [PATCH 28/34] nit --- sky/adaptors/cloudflare.py | 3 +-- sky/check.py | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/sky/adaptors/cloudflare.py b/sky/adaptors/cloudflare.py index 1685d0babe9..5f7e8024eeb 100644 --- a/sky/adaptors/cloudflare.py +++ b/sky/adaptors/cloudflare.py @@ -54,7 +54,7 @@ def session(): @import_package def resource(resource_name: str, **kwargs): """Create a Cloudflare resource. - + Args: resource_name: Cloudflare resource name (e.g., 's3'). kwargs: Other options. @@ -124,7 +124,6 @@ def create_endpoint(): def check_credentials() -> Tuple[bool, Optional[str]]: - """Checks if the user has access credentials to Cloudflare R2. Returns: diff --git a/sky/check.py b/sky/check.py index b4574e93bfc..463cde9c9fb 100644 --- a/sky/check.py +++ b/sky/check.py @@ -53,7 +53,7 @@ def check(quiet: bool = False) -> None: def get_cloud_credential_file_mounts() -> Dict[str, str]: """Returns the files necessary to access all enabled clouds. - + Returns a dictionary that will be added to a task's file mounts and a list of patterns that will be excluded (used as rsync_exclude). """ From efb1d7f0029ddeb66f42c1d48e9811b271027736 Mon Sep 17 00:00:00 2001 From: Doyoung Kim Date: Tue, 9 May 2023 21:03:27 -0700 Subject: [PATCH 29/34] remove unused code --- sky/global_user_state.py | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/sky/global_user_state.py b/sky/global_user_state.py index 1c45685d920..ea828a919e6 100644 --- a/sky/global_user_state.py +++ b/sky/global_user_state.py @@ -664,26 +664,6 @@ def get_enabled_clouds() -> List[clouds.Cloud]: return enabled_clouds -def get_enabled_clouds_str() -> List[str]: - # Currently, 'clouds' only support cloud types with - # computing instances. The following is to temporarily - # support R2 for get_enabled_clouds - rows = _DB.cursor.execute('SELECT value FROM config WHERE key = ?', - (_ENABLED_CLOUDS_KEY,)) - ret = [] - for (value,) in rows: - ret = json.loads(value) - break - enabled_clouds: List[str] = [] - for c in ret: - cloud = clouds.CLOUD_REGISTRY.from_str(c) - if cloud is not None: - enabled_clouds.append(str(cloud)) - if os.path.exists(os.path.expanduser(cloudflare.ACCOUNT_ID_PATH)): - enabled_clouds.append('Cloudflare') - return enabled_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))) From 2a01678a85ffe9c09ff3fa2ac80302fbd30bed4b Mon Sep 17 00:00:00 2001 From: Doyoung Kim Date: Tue, 9 May 2023 21:50:37 -0700 Subject: [PATCH 30/34] remove unused --- sky/global_user_state.py | 1 - 1 file changed, 1 deletion(-) diff --git a/sky/global_user_state.py b/sky/global_user_state.py index ea828a919e6..4bd6ff67912 100644 --- a/sky/global_user_state.py +++ b/sky/global_user_state.py @@ -22,7 +22,6 @@ from sky import clouds from sky.utils import db_utils from sky.utils import common_utils -from sky.adaptors import cloudflare if typing.TYPE_CHECKING: from sky import backends From c20251d1a3305fba53731c376ee5a52641b6634b Mon Sep 17 00:00:00 2001 From: Doyoung Kim Date: Sat, 20 May 2023 05:18:23 +0000 Subject: [PATCH 31/34] update with enabled_storage_clouds --- sky/check.py | 2 +- sky/data/storage.py | 25 +++++++++++++++++++++++-- sky/exceptions.py | 5 ----- sky/global_user_state.py | 20 +++++++++++++++++++- sky/task.py | 15 --------------- 5 files changed, 43 insertions(+), 24 deletions(-) diff --git a/sky/check.py b/sky/check.py index 463cde9c9fb..5d1f2de62cc 100644 --- a/sky/check.py +++ b/sky/check.py @@ -66,7 +66,7 @@ def get_cloud_credential_file_mounts() -> Dict[str, str]: # as only clouds with computing instances are marked # as enabled by skypilot. This will be removed when # cloudflare/r2 is added as a 'cloud'. - r2_is_enabled, _ = cloud = cloudflare.check_credentials() + r2_is_enabled, _ = cloudflare.check_credentials() if r2_is_enabled: r2_credential_mounts = cloudflare.get_credential_file_mounts() file_mounts.update(r2_credential_mounts) diff --git a/sky/data/storage.py b/sky/data/storage.py index 33044c2006c..987ba27b4c4 100644 --- a/sky/data/storage.py +++ b/sky/data/storage.py @@ -42,8 +42,6 @@ # R2 to be an option as preferred store type STORE_ENABLED_CLOUDS = [clouds.AWS(), clouds.GCP()] -STORE_TYPE_TO_CLOUD_TYPE = {'s3': 'AWS', 'gcs': 'GCP', 'r2': 'Cloudflare'} - # Maximum number of concurrent rsync upload processes _MAX_CONCURRENT_UPLOADS = 32 @@ -914,6 +912,14 @@ 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. Enable AWS to fix.') + @classmethod def validate_name(cls, name) -> str: """Validates the name of the S3 store. @@ -1289,6 +1295,13 @@ 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: gs\' specified, but ' \ + 'GCP access is disabled. Enable GCP to fix.') @classmethod def validate_name(cls, name) -> str: @@ -1691,7 +1704,15 @@ 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' not in enabled_storage_clouds: + with ux_utils.print_exception_no_traceback(): + raise exceptions.ResourcesUnavailableError( + 'Storage \'store: r2\' specified, but ' \ + 'Cloudflare access is disabled. Enable Cloudflare to fix.') def initialize(self): """Initializes the R2 store object on the cloud. diff --git a/sky/exceptions.py b/sky/exceptions.py index 8ea4fd2dcfe..0211cb81efa 100644 --- a/sky/exceptions.py +++ b/sky/exceptions.py @@ -209,8 +209,3 @@ class ClusterOwnerIdentityMismatchError(Exception): class NoCloudAccessError(Exception): """Raised when all clouds are disabled.""" pass - - -class CloudDisabledError(Exception): - """Raised when attempted cloud is disabled""" - pass diff --git a/sky/global_user_state.py b/sky/global_user_state.py index 4bd6ff67912..efbf61377bb 100644 --- a/sky/global_user_state.py +++ b/sky/global_user_state.py @@ -20,8 +20,9 @@ import colorama from sky import clouds -from sky.utils import db_utils +from sky.adaptors import cloudflare from sky.utils import common_utils +from sky.utils import db_utils if typing.TYPE_CHECKING: from sky import backends @@ -663,6 +664,23 @@ 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() + # TODO: Remove clouds.IBM from below condition as IBM COS gets integrated + enabled_storage_clouds = [ + str(cloud) + for cloud in enabled_clouds + if not isinstance(cloud, clouds.Lambda) and + not isinstance(cloud, clouds.IBM) + ] + r2_is_enabled, _ = cloudflare.check_credentials() + if r2_is_enabled: + enabled_storage_clouds.append('Cloudflare') + 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 28015672236..972a1e95e37 100644 --- a/sky/task.py +++ b/sky/task.py @@ -17,7 +17,6 @@ from sky.skylet import constants from sky.utils import schemas from sky.utils import ux_utils -from sky.adaptors import cloudflare if typing.TYPE_CHECKING: from sky import resources as resources_lib @@ -296,24 +295,10 @@ def from_yaml(yaml_path: str) -> 'Task': task_storage_mounts: Dict[str, storage_lib.Storage] = {} all_storages = fm_storages - enabled_clouds = global_user_state.get_enabled_clouds() - enabled_clouds = [str(cloud) for cloud in enabled_clouds] - r2_is_enabled, _ = cloudflare.check_credentials() - if r2_is_enabled: - enabled_clouds.append('r2') for storage in all_storages: mount_path = storage[0] assert mount_path, \ 'Storage mount path cannot be empty.' - store_type = storage[1]['store'] - if store_type: - cloud_type = storage_lib.STORE_TYPE_TO_CLOUD_TYPE[store_type] - if cloud_type not in enabled_clouds: - with ux_utils.print_exception_no_traceback(): - raise exceptions.CloudDisabledError( - f'Storage \'store:{store_type}\' specified, but ' - f'\'{cloud_type}\' access is disabled. Enable ' - f'\'{cloud_type}\' to fix.') try: storage_obj = storage_lib.Storage.from_yaml_config(storage[1]) except exceptions.StorageSourceError as e: From 935bb09850a02b540b18e0e30005ba012605e696 Mon Sep 17 00:00:00 2001 From: Doyoung Kim Date: Tue, 23 May 2023 05:02:14 +0000 Subject: [PATCH 32/34] update STORE_ENABLED_CLOUDS to List[str] --- sky/adaptors/cloudflare.py | 1 + sky/data/storage.py | 4 +++- sky/global_user_state.py | 12 ++++++------ sky/task.py | 5 +++-- 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/sky/adaptors/cloudflare.py b/sky/adaptors/cloudflare.py index 5f7e8024eeb..6f511ba8e1a 100644 --- a/sky/adaptors/cloudflare.py +++ b/sky/adaptors/cloudflare.py @@ -13,6 +13,7 @@ ACCOUNT_ID_PATH = '~/.cloudflare/accountid' AWS_R2_PROFILE_PATH = '~/.aws/credentials' R2_PROFILE_NAME = 'r2' +NAME = 'Cloudflare' def import_package(func): diff --git a/sky/data/storage.py b/sky/data/storage.py index 987ba27b4c4..28413811cf6 100644 --- a/sky/data/storage.py +++ b/sky/data/storage.py @@ -40,7 +40,9 @@ # 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 = [clouds.AWS(), clouds.GCP()] +STORE_ENABLED_CLOUDS: List[str] = [ + str(clouds.AWS()), str(clouds.GCP()), cloudflare.NAME +] # Maximum number of concurrent rsync upload processes _MAX_CONCURRENT_UPLOADS = 32 diff --git a/sky/global_user_state.py b/sky/global_user_state.py index efbf61377bb..ca858ee45bd 100644 --- a/sky/global_user_state.py +++ b/sky/global_user_state.py @@ -21,6 +21,7 @@ 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 @@ -668,16 +669,15 @@ 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() - # TODO: Remove clouds.IBM from below condition as IBM COS gets integrated + enabled_clouds = [str(cloud) for cloud in enabled_clouds] + enabled_storage_clouds = [ - str(cloud) - for cloud in enabled_clouds - if not isinstance(cloud, clouds.Lambda) and - not isinstance(cloud, clouds.IBM) + 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') + enabled_storage_clouds.append(cloudflare.NAME) return enabled_storage_clouds diff --git a/sky/task.py b/sky/task.py index 972a1e95e37..0f3b6d1e4e1 100644 --- a/sky/task.py +++ b/sky/task.py @@ -690,8 +690,9 @@ def get_preferred_store_type(self) -> storage_lib.StoreType: for cloud in storage_lib.STORE_ENABLED_CLOUDS: for enabled_cloud in enabled_clouds: - if cloud.is_same_cloud(enabled_cloud): - storage_cloud = cloud + str_enabled_cloud = str(enabled_cloud) + if str_enabled_cloud == cloud: + storage_cloud = enabled_cloud break if storage_cloud is None: raise ValueError('No available cloud to mount storage.') From 12d9f00d0b61e35ee58c07d80f31d7723bf49e05 Mon Sep 17 00:00:00 2001 From: Doyoung Kim Date: Thu, 25 May 2023 04:11:06 +0000 Subject: [PATCH 33/34] update on message and task.py --- sky/data/storage.py | 19 ++++++++++++++----- sky/task.py | 11 +++-------- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/sky/data/storage.py b/sky/data/storage.py index 28413811cf6..6cdd24cd0f9 100644 --- a/sky/data/storage.py +++ b/sky/data/storage.py @@ -920,7 +920,10 @@ def _validate(self): with ux_utils.print_exception_no_traceback(): raise exceptions.ResourcesUnavailableError( 'Storage \'store: s3\' specified, but ' \ - 'AWS access is disabled. Enable AWS to fix.') + '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: @@ -1302,8 +1305,11 @@ def _validate(self): if str(clouds.GCP()) not in enabled_storage_clouds: with ux_utils.print_exception_no_traceback(): raise exceptions.ResourcesUnavailableError( - 'Storage \'store: gs\' specified, but ' \ - 'GCP access is disabled. Enable GCP to fix.') + '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: @@ -1710,11 +1716,14 @@ def _validate(self): 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' not in 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 access is disabled. Enable Cloudflare to fix.') + '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/task.py b/sky/task.py index 0f3b6d1e4e1..96daee3d67b 100644 --- a/sky/task.py +++ b/sky/task.py @@ -686,14 +686,9 @@ 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_clouds = global_user_state.get_enabled_clouds() - - for cloud in storage_lib.STORE_ENABLED_CLOUDS: - for enabled_cloud in enabled_clouds: - str_enabled_cloud = str(enabled_cloud) - if str_enabled_cloud == cloud: - storage_cloud = enabled_cloud - break + enabled_storage_clouds = \ + global_user_state.get_enabled_storage_clouds() + storage_cloud = enabled_storage_clouds[0] if storage_cloud is None: raise ValueError('No available cloud to mount storage.') store_type = storage_lib.get_storetype_from_cloud(storage_cloud) From 79be0b8e3fc23c57fedbdc59dce2db36f32b4983 Mon Sep 17 00:00:00 2001 From: Doyoung Kim Date: Thu, 25 May 2023 04:15:51 +0000 Subject: [PATCH 34/34] nit --- sky/task.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sky/task.py b/sky/task.py index 96daee3d67b..377073661c8 100644 --- a/sky/task.py +++ b/sky/task.py @@ -688,7 +688,8 @@ def get_preferred_store_type(self) -> storage_lib.StoreType: backend_utils.check_public_cloud_enabled() enabled_storage_clouds = \ global_user_state.get_enabled_storage_clouds() - storage_cloud = enabled_storage_clouds[0] + if enabled_storage_clouds: + storage_cloud = enabled_storage_clouds[0] if storage_cloud is None: raise ValueError('No available cloud to mount storage.') store_type = storage_lib.get_storetype_from_cloud(storage_cloud)