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.

Sign up for GitHub

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

Jump to bottom

Replace deprecated azure.common usages #3466

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Replace deprecated azure.common usages #3466

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
933b0a0
use cli to retrieve info
Programmerino Apr 23, 2024
fa84965
refactor w/ common implementation
Programmerino Apr 25, 2024
03abe9b
add error handling, stderr output
Programmerino May 2, 2024
b0adc77
Refactor azure.py to use subprocess_utils and ux_utils for error hand…
Programmerino May 26, 2024
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
21 changes: 15 additions & 6 deletions sky/adaptors/azure.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@

# pylint: disable=import-outside-toplevel
import functools
import json
import subprocess
import threading

from sky.adaptors import common
from sky.utils.subprocess_utils import run
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
from sky.utils.subprocess_utils import run
from sky.utils import subprocess_utils

nit: import package and modules only; https://google.github.io/styleguide/pyguide.html#2144-decision


azure = common.LazyImport(
'azure',
Expand All @@ -14,19 +17,25 @@

_session_creation_lock = threading.RLock()

# 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('az account show -o json',
stdout=subprocess.PIPE,
stderr=subprocess.DEVNULL).stdout.decode())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we expose the stderr here and add proper error handling? maybe raise an runtime error if json parse error and print out the error message



@common.load_lazy_modules(modules=_LAZY_MODULES)
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)
Expand Down
4 changes: 2 additions & 2 deletions sky/skylet/providers/azure/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@
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.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
Expand Down Expand Up @@ -51,7 +51,7 @@ def _configure_resource_group(config):
# 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()
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
Expand Down
Loading