From 933b0a04f5b92853120613dcf970ab6bc9d8b187 Mon Sep 17 00:00:00 2001 From: Davis Davalos-DeLosh Date: Tue, 23 Apr 2024 01:08:14 -0600 Subject: [PATCH 1/4] use cli to retrieve info --- sky/adaptors/azure.py | 27 +++++++++++++++++++++------ sky/skylet/providers/azure/config.py | 10 ++++++---- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/sky/adaptors/azure.py b/sky/adaptors/azure.py index 44618a8f64f..396fce34ff2 100644 --- a/sky/adaptors/azure.py +++ b/sky/adaptors/azure.py @@ -2,6 +2,8 @@ # pylint: disable=import-outside-toplevel import functools +import json +import subprocess import threading from sky.adaptors import common @@ -15,18 +17,31 @@ _session_creation_lock = threading.RLock() -@common.load_lazy_modules(modules=_LAZY_MODULES) +def _run_output(cmd): + proc = subprocess.run(cmd, + shell=True, + check=True, + stderr=subprocess.PIPE, + stdout=subprocess.PIPE) + return proc.stdout.decode('ascii') + + +# There is no programmatic way to get current account details anymore. +# https://github.com/Azure/azure-sdk-for-python/issues/21561 + + +def _get_account(): + return json.loads(_run_output('az account show -o json')) + + def get_subscription_id() -> str: """Get the default subscription id.""" - from azure.common import credentials - return credentials.get_cli_profile().get_subscription_id() + return _get_account()['id'] -@common.load_lazy_modules(modules=_LAZY_MODULES) def get_current_account_user() -> str: """Get the default account user.""" - from azure.common import credentials - return credentials.get_cli_profile().get_current_account_user() + return _get_account()['user']['name'] @common.load_lazy_modules(modules=_LAZY_MODULES) diff --git a/sky/skylet/providers/azure/config.py b/sky/skylet/providers/azure/config.py index 0c1827a1141..dd482cc526b 100644 --- a/sky/skylet/providers/azure/config.py +++ b/sky/skylet/providers/azure/config.py @@ -6,10 +6,9 @@ import time from typing import Any, Callable -from azure.common.credentials import get_cli_profile from azure.identity import AzureCliCredential from azure.mgmt.network import NetworkManagementClient -from azure.mgmt.resource import ResourceManagementClient +from azure.mgmt.resource import ResourceManagementClient, SubscriptionClient from azure.mgmt.resource.resources.models import DeploymentMode from sky.utils import common_utils @@ -50,12 +49,15 @@ def _configure_resource_group(config): # TODO: look at availability sets # https://docs.microsoft.com/en-us/azure/virtual-machines/windows/tutorial-availability-sets subscription_id = config["provider"].get("subscription_id") - if subscription_id is None: - subscription_id = get_cli_profile().get_subscription_id() # Increase the timeout to fix the Azure get-access-token (used by ray azure # node_provider) timeout issue. # Tracked in https://github.com/Azure/azure-cli/issues/20404#issuecomment-1249575110 credentials = AzureCliCredential(process_timeout=30) + + if subscription_id is None: + subscription_client = SubscriptionClient(credentials) + subscription_id = next(subscription_client.subscriptions.list()).subscription_id + resource_client = ResourceManagementClient(credentials, subscription_id) config["provider"]["subscription_id"] = subscription_id logger.info("Using subscription id: %s", subscription_id) From fa849654d2f82b0a908b58c7097e1d321a87dc38 Mon Sep 17 00:00:00 2001 From: Davis Davalos-DeLosh Date: Thu, 25 Apr 2024 15:23:27 -0600 Subject: [PATCH 2/4] refactor w/ common implementation --- sky/adaptors/azure.py | 16 +++++----------- sky/skylet/providers/azure/config.py | 10 ++++------ 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/sky/adaptors/azure.py b/sky/adaptors/azure.py index 396fce34ff2..c4324f499d8 100644 --- a/sky/adaptors/azure.py +++ b/sky/adaptors/azure.py @@ -7,6 +7,7 @@ import threading from sky.adaptors import common +from sky.utils.subprocess_utils import run azure = common.LazyImport( 'azure', @@ -16,22 +17,15 @@ _session_creation_lock = threading.RLock() - -def _run_output(cmd): - proc = subprocess.run(cmd, - shell=True, - check=True, - stderr=subprocess.PIPE, - stdout=subprocess.PIPE) - return proc.stdout.decode('ascii') - - # There is no programmatic way to get current account details anymore. # https://github.com/Azure/azure-sdk-for-python/issues/21561 def _get_account(): - return json.loads(_run_output('az account show -o json')) + return json.loads( + run('az account show -o json', + stdout=subprocess.PIPE, + stderr=subprocess.DEVNULL).stdout.decode()) def get_subscription_id() -> str: diff --git a/sky/skylet/providers/azure/config.py b/sky/skylet/providers/azure/config.py index dd482cc526b..a4f3fb37536 100644 --- a/sky/skylet/providers/azure/config.py +++ b/sky/skylet/providers/azure/config.py @@ -8,10 +8,11 @@ from azure.identity import AzureCliCredential from azure.mgmt.network import NetworkManagementClient -from azure.mgmt.resource import ResourceManagementClient, SubscriptionClient +from azure.mgmt.resource import ResourceManagementClient from azure.mgmt.resource.resources.models import DeploymentMode from sky.utils import common_utils +from sky.adaptors.azure import get_subscription_id UNIQUE_ID_LEN = 4 _WAIT_NSG_CREATION_NUM_TIMEOUT_SECONDS = 600 @@ -49,15 +50,12 @@ def _configure_resource_group(config): # TODO: look at availability sets # https://docs.microsoft.com/en-us/azure/virtual-machines/windows/tutorial-availability-sets subscription_id = config["provider"].get("subscription_id") + if subscription_id is None: + subscription_id = get_subscription_id() # Increase the timeout to fix the Azure get-access-token (used by ray azure # node_provider) timeout issue. # Tracked in https://github.com/Azure/azure-cli/issues/20404#issuecomment-1249575110 credentials = AzureCliCredential(process_timeout=30) - - if subscription_id is None: - subscription_client = SubscriptionClient(credentials) - subscription_id = next(subscription_client.subscriptions.list()).subscription_id - resource_client = ResourceManagementClient(credentials, subscription_id) config["provider"]["subscription_id"] = subscription_id logger.info("Using subscription id: %s", subscription_id) From 03abe9b969088a11fc9fa2c8f8ce13eac3e04acf Mon Sep 17 00:00:00 2001 From: Davis Davalos-DeLosh Date: Thu, 2 May 2024 13:37:44 -0600 Subject: [PATCH 3/4] add error handling, stderr output --- sky/adaptors/azure.py | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/sky/adaptors/azure.py b/sky/adaptors/azure.py index c4324f499d8..ba7f9709471 100644 --- a/sky/adaptors/azure.py +++ b/sky/adaptors/azure.py @@ -22,10 +22,25 @@ def _get_account(): - return json.loads( - run('az account show -o json', - stdout=subprocess.PIPE, - stderr=subprocess.DEVNULL).stdout.decode()) + result = run('az account show -o json', + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + + if result.returncode != 0: + error_message = result.stderr.decode() + print(f'Error executing command: {error_message}') + raise RuntimeError( + 'Failed to execute az account show -o json command.') + + try: + return json.loads(result.stdout.decode()) + except json.JSONDecodeError as e: + error_message = result.stderr.decode() + print(f'JSON parsing error: {e.msg}') + raise RuntimeError( + f'Failed to parse JSON output. Error: {e.msg}\n' + f'Command Error: {error_message}' + ) from e def get_subscription_id() -> str: From b0adc7778bed7929f45db2f26dcb8f3ff1fc1b44 Mon Sep 17 00:00:00 2001 From: Davis Davalos-DeLosh Date: Sat, 25 May 2024 22:51:44 -0600 Subject: [PATCH 4/4] Refactor azure.py to use subprocess_utils and ux_utils for error handling and output handling --- sky/adaptors/azure.py | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/sky/adaptors/azure.py b/sky/adaptors/azure.py index ba7f9709471..8d1d66bf736 100644 --- a/sky/adaptors/azure.py +++ b/sky/adaptors/azure.py @@ -7,7 +7,8 @@ import threading from sky.adaptors import common -from sky.utils.subprocess_utils import run +from sky.utils import subprocess_utils +from sky.utils import ux_utils azure = common.LazyImport( 'azure', @@ -22,25 +23,25 @@ def _get_account(): - result = run('az account show -o json', - stdout=subprocess.PIPE, - stderr=subprocess.PIPE) + result = subprocess_utils.run('az account show -o json', + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) if result.returncode != 0: error_message = result.stderr.decode() - print(f'Error executing command: {error_message}') - raise RuntimeError( - 'Failed to execute az account show -o json command.') + with ux_utils.print_exception_no_traceback(): + raise RuntimeError( + 'Failed to execute az account show -o json command. ' + f'Error: {error_message}') try: return json.loads(result.stdout.decode()) except json.JSONDecodeError as e: error_message = result.stderr.decode() - print(f'JSON parsing error: {e.msg}') - raise RuntimeError( - f'Failed to parse JSON output. Error: {e.msg}\n' - f'Command Error: {error_message}' - ) from e + with ux_utils.print_exception_no_traceback(): + raise RuntimeError( + 'Failed to parse the output of az account show -o json ' + f'command. Error: {error_message}') from e def get_subscription_id() -> str: