Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[UX] Optimizing error message when disabled cloud storage attempted #1858

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
2d9a3b2
ux_optimization_disabled_clouds
landscapepainter Apr 13, 2023
2380611
support get_enabled_clouds_str()
landscapepainter Apr 13, 2023
6e3cd17
fix return type
landscapepainter Apr 13, 2023
1b2ae82
adding new line eof
landscapepainter Apr 13, 2023
3aa6a35
fix error messages
landscapepainter Apr 13, 2023
3b905da
update for non-cloud-storage
landscapepainter Apr 14, 2023
845ab23
remove unused
landscapepainter Apr 15, 2023
0d5c288
Merge branch 'disabled_cloud_ux_optimize' of https://github.com/lands…
landscapepainter Apr 15, 2023
3780f6f
update check.py, cloudflare.py
landscapepainter Apr 15, 2023
8f06b45
format
landscapepainter Apr 15, 2023
f9039b1
format
landscapepainter Apr 15, 2023
bbf5093
format
landscapepainter Apr 15, 2023
2a82a09
update fix
landscapepainter Apr 15, 2023
7540085
ux_optimization_disabled_clouds
landscapepainter Apr 13, 2023
66ad186
support get_enabled_clouds_str()
landscapepainter Apr 13, 2023
da89eb8
fix return type
landscapepainter Apr 13, 2023
af9fdb3
adding new line eof
landscapepainter Apr 13, 2023
6737596
fix error messages
landscapepainter Apr 13, 2023
6299804
remove unused
landscapepainter Apr 15, 2023
d19e3fd
update for non-cloud-storage
landscapepainter Apr 14, 2023
6ee3952
rebasing
landscapepainter Apr 15, 2023
d1ca1f9
error fix
landscapepainter Apr 15, 2023
ec04950
format
landscapepainter Apr 15, 2023
256b6f3
fix
landscapepainter Apr 15, 2023
cf874da
format
landscapepainter Apr 15, 2023
bc02cb7
fix
landscapepainter Apr 20, 2023
5acdbbe
format
landscapepainter Apr 20, 2023
0c2b2b1
format
landscapepainter Apr 20, 2023
d79ebee
Merge branch 'master' into disabled_cloud_ux_optimize
landscapepainter May 5, 2023
4f28c4a
nit
landscapepainter May 5, 2023
7f57e6d
nit
landscapepainter May 5, 2023
efb1d7f
remove unused code
landscapepainter May 10, 2023
2a01678
remove unused
landscapepainter May 10, 2023
c20251d
update with enabled_storage_clouds
landscapepainter May 20, 2023
935bb09
update STORE_ENABLED_CLOUDS to List[str]
landscapepainter May 23, 2023
16fb1ea
Merge branch 'master' into disabled_cloud_ux_optimize
landscapepainter May 23, 2023
12d9f00
update on message and task.py
landscapepainter May 25, 2023
b95d943
Merge branch 'disabled_cloud_ux_optimize' of https://github.com/lands…
landscapepainter May 25, 2023
79be0b8
nit
landscapepainter May 25, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions sky/adaptors/cloudflare.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
AWS_R2_PROFILE_PATH = '~/.aws/credentials'
R2_PROFILE_NAME = 'r2'
_INDENT_PREFIX = ' '
NAME = 'Cloudflare'


def import_package(func):
Expand Down
36 changes: 35 additions & 1 deletion sky/data/storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -912,6 +914,17 @@ 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 ' \
landscapepainter marked this conversation as resolved.
Show resolved Hide resolved
'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.
Expand Down Expand Up @@ -1287,6 +1300,16 @@ 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:
Expand Down Expand Up @@ -1689,7 +1712,18 @@ 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.
Expand Down
20 changes: 19 additions & 1 deletion sky/global_user_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@
import colorama

from sky import clouds
from sky.utils import db_utils
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

if typing.TYPE_CHECKING:
from sky import backends
Expand Down Expand Up @@ -663,6 +665,22 @@ 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)))
Expand Down
11 changes: 4 additions & 7 deletions sky/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -686,13 +686,10 @@ 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:
if cloud.is_same_cloud(enabled_cloud):
storage_cloud = cloud
break
enabled_storage_clouds = \
global_user_state.get_enabled_storage_clouds()
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)
Expand Down