From 49d2bd5e9b27b97b79023dd48dbf69ae6a36d881 Mon Sep 17 00:00:00 2001 From: Zongheng Yang Date: Wed, 14 Feb 2024 14:16:31 -0800 Subject: [PATCH 1/3] [UX] `sky check`: show enabled clouds in a table --- sky/check.py | 67 +++++++++++++++----------- sky/provision/vsphere/vsphere_utils.py | 2 +- sky/utils/subprocess_utils.py | 8 +-- 3 files changed, 44 insertions(+), 33 deletions(-) diff --git a/sky/check.py b/sky/check.py index b998bac4ce0..c10e80bea8b 100644 --- a/sky/check.py +++ b/sky/check.py @@ -1,7 +1,8 @@ """Credential checks: check cloud credentials and enable clouds.""" -from typing import Dict, Iterable, Optional +from typing import Dict, Iterable, Optional, Tuple import click +import rich from sky import clouds from sky import global_user_state @@ -14,20 +15,22 @@ def check(quiet: bool = False, verbose: bool = False) -> None: echo('Checking credentials to enable clouds for SkyPilot.') enabled_clouds = [] - for cloud in clouds.CLOUD_REGISTRY.values(): + + def check_one_cloud(cloud_tuple: Tuple[str, clouds.Cloud]) -> None: + cloud_repr, cloud = cloud_tuple if not isinstance(cloud, clouds.Local): - echo(f' Checking {cloud}...', nl=False) + echo(f' Checking {cloud_repr}...', nl=False) ok, reason = cloud.check_credentials() echo('\r', nl=False) status_msg = 'enabled' if ok else 'disabled' status_color = 'green' if ok else 'red' if not isinstance(cloud, clouds.Local): echo(' ' + click.style( - f'{cloud}: {status_msg}', fg=status_color, bold=True) + + f'{cloud_repr}: {status_msg}', fg=status_color, bold=True) + ' ' * 30) if ok: - enabled_clouds.append(str(cloud)) - if verbose: + enabled_clouds.append(cloud_repr) + if verbose and cloud is not cloudflare: activated_account = cloud.get_current_user_identity_str() if activated_account is not None: echo(f' Activated account: {activated_account}') @@ -36,23 +39,15 @@ def check(quiet: bool = False, verbose: bool = False) -> None: else: echo(f' Reason: {reason}') - # Currently, clouds.CLOUD_REGISTRY.values() does not - # support r2 as only clouds with computing instances - # are added as 'cloud'. This will be removed when - # cloudflare/r2 is added as a 'cloud'. - cloud = 'Cloudflare (for R2 object store)' - echo(f' Checking {cloud}...', nl=False) - r2_is_enabled, reason = cloudflare.check_credentials() - echo('\r', nl=False) - status_msg = 'enabled' if r2_is_enabled else 'disabled' - status_color = 'green' if r2_is_enabled else 'red' - echo(' ' + - click.style(f'{cloud}: {status_msg}', fg=status_color, bold=True) + - ' ' * 30) - if not r2_is_enabled: - echo(f' Reason: {reason}') + clouds_to_check = [ + (repr(cloud), cloud) for cloud in clouds.CLOUD_REGISTRY.values() + ] + clouds_to_check.append(('Cloudflare, for R2 object store', cloudflare)) + + for cloud_tuple in sorted(clouds_to_check): + check_one_cloud(cloud_tuple) - if len(enabled_clouds) == 0 and not r2_is_enabled: + if len(enabled_clouds) == 0: click.echo( click.style( 'No cloud is enabled. SkyPilot will not be able to run any ' @@ -61,14 +56,28 @@ def check(quiet: bool = False, verbose: bool = False) -> None: bold=True)) raise SystemExit() else: - echo('\nSkyPilot will use only the enabled clouds to run tasks. ' - 'To change this, configure cloud credentials, ' - 'and run ' + click.style('sky check', bold=True) + '.' - '\n' + click.style( - 'If any problems remain, please file an issue at ' - 'https://github.com/skypilot-org/skypilot/issues/new', - dim=True)) + echo( + click.style( + '\nTo enable a cloud, follow the hints above and rerun: ', + dim=True) + click.style('sky check', bold=True) + '\n' + + click.style( + 'If any problems remain, open an issue at: ' + 'https://github.com/skypilot-org/skypilot/issues/new', + dim=True)) + + # Pretty print for UX. + if not quiet: + enabled_clouds_str = '\n :heavy_check_mark: '.join( + [''] + sorted(enabled_clouds)) + rich.print('\n[green]:tada: Enabled clouds :tada:' + f'{enabled_clouds_str}[/green]') + # Cloudflare is not a real cloud in clouds.CLOUD_REGISTRY, and should not be + # inserted into the DB (otherwise `sky launch` and other code would error + # out when it's trying to look it up in the registry). + enabled_clouds = [ + cloud for cloud in enabled_clouds if not cloud.startswith('Cloudflare') + ] global_user_state.set_enabled_clouds(enabled_clouds) diff --git a/sky/provision/vsphere/vsphere_utils.py b/sky/provision/vsphere/vsphere_utils.py index cb92bb3ef3d..375a1a157c5 100644 --- a/sky/provision/vsphere/vsphere_utils.py +++ b/sky/provision/vsphere/vsphere_utils.py @@ -320,7 +320,7 @@ def get_vsphere_credentials(name=None): """ credential_path = os.path.expanduser(CREDENTIALS_PATH) assert os.path.exists( - credential_path), f' Missing credential file at {credential_path}.' + credential_path), f'Missing credential file at {credential_path}.' with open(credential_path, 'r') as file: credential = yaml.safe_load(file) vcenters = credential['vcenters'] diff --git a/sky/utils/subprocess_utils.py b/sky/utils/subprocess_utils.py index 73b4ecb4e6f..f45613c44a3 100644 --- a/sky/utils/subprocess_utils.py +++ b/sky/utils/subprocess_utils.py @@ -53,9 +53,11 @@ def get_parallel_threads() -> int: def run_in_parallel(func: Callable, args: Iterable[Any]) -> List[Any]: """Run a function in parallel on a list of arguments. - The function should raise a CommandError if the command fails. - Returns a list of the return values of the function func, in the same order - as the arguments. + The function 'func' should raise a CommandError if the command fails. + + Returns: + A list of the return values of the function func, in the same order as the + arguments. """ # Reference: https://stackoverflow.com/questions/25790279/python-multiprocessing-early-termination # pylint: disable=line-too-long with pool.ThreadPool(processes=get_parallel_threads()) as p: From a0c204185b34bb90bff330b0f49cb59188ab8156 Mon Sep 17 00:00:00 2001 From: Zongheng Yang Date: Wed, 14 Feb 2024 14:55:09 -0800 Subject: [PATCH 2/3] Change colors --- sky/check.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/sky/check.py b/sky/check.py index c10e80bea8b..5083ba21fba 100644 --- a/sky/check.py +++ b/sky/check.py @@ -23,10 +23,9 @@ def check_one_cloud(cloud_tuple: Tuple[str, clouds.Cloud]) -> None: ok, reason = cloud.check_credentials() echo('\r', nl=False) status_msg = 'enabled' if ok else 'disabled' - status_color = 'green' if ok else 'red' + styles = {'fg': 'green', 'bold': False} if ok else {'dim': True} if not isinstance(cloud, clouds.Local): - echo(' ' + click.style( - f'{cloud_repr}: {status_msg}', fg=status_color, bold=True) + + echo(' ' + click.style(f'{cloud_repr}: {status_msg}', **styles) + ' ' * 30) if ok: enabled_clouds.append(cloud_repr) From a620bfa8e35eeab1319dbbb8eac6cf29778892b1 Mon Sep 17 00:00:00 2001 From: Zongheng Yang Date: Fri, 16 Feb 2024 10:44:09 -0800 Subject: [PATCH 3/3] Link --- sky/check.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sky/check.py b/sky/check.py index 5083ba21fba..84bb19c8da7 100644 --- a/sky/check.py +++ b/sky/check.py @@ -60,8 +60,8 @@ def check_one_cloud(cloud_tuple: Tuple[str, clouds.Cloud]) -> None: '\nTo enable a cloud, follow the hints above and rerun: ', dim=True) + click.style('sky check', bold=True) + '\n' + click.style( - 'If any problems remain, open an issue at: ' - 'https://github.com/skypilot-org/skypilot/issues/new', + 'If any problems remain, refer to detailed docs at: ' + 'https://skypilot.readthedocs.io/en/latest/getting-started/installation.html', # pylint: disable=line-too-long dim=True)) # Pretty print for UX.