From f5657de8795d6b18e855179b0ea4395ba9a817b3 Mon Sep 17 00:00:00 2001 From: Matt Brown Date: Mon, 30 Jan 2023 11:30:03 -0500 Subject: [PATCH 01/24] Adding in some functionality to use different clouds This support is needed for the air gapped environments. There are three ways to add a new cloud environment. These were all taken from examples in the v1 sdk. 1) The SDK will look for a default configuration file and try to find cloud environments in there. 2) If you set an environment variable called ARM_METADATA_URL, it will look there for cloud configurations. If you do not set this, it will use a default URL in the _azure_environments.py file to find them. 3) The SDK exposes two new functions, add_cloud which will add the new configuration to the configuration file mentioned in #1, and update_cloud which will update the added configuration. --- sdk/ml/azure-ai-ml/azure/ai/ml/__init__.py | 6 + .../azure/ai/ml/_azure_environments.py | 134 +++++++++++++++++- .../azure/ai/ml/entities/_manage_clouds.py | 7 + 3 files changed, 145 insertions(+), 2 deletions(-) create mode 100644 sdk/ml/azure-ai-ml/azure/ai/ml/entities/_manage_clouds.py diff --git a/sdk/ml/azure-ai-ml/azure/ai/ml/__init__.py b/sdk/ml/azure-ai-ml/azure/ai/ml/__init__.py index 9499e342c21a..803b1424fe10 100644 --- a/sdk/ml/azure-ai-ml/azure/ai/ml/__init__.py +++ b/sdk/ml/azure-ai-ml/azure/ai/ml/__init__.py @@ -32,6 +32,10 @@ load_workspace, load_workspace_connection, ) +from .entities._manage_clouds import ( + add_cloud, + update_cloud, +) module_logger = logging.getLogger(__name__) initialize_logger_info(module_logger, terminator="\n") @@ -63,6 +67,8 @@ "load_workspace", "load_registry", "load_workspace_connection", + "add_cloud", + "update_cloud", ] __version__ = VERSION diff --git a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py index 325173b229c7..3c7c25e0f40d 100644 --- a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py +++ b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py @@ -4,8 +4,10 @@ """Metadata to interact with different Azure clouds.""" +import configparser import logging import os +import sys from typing import Dict, Optional from azure.ai.ml._utils.utils import _get_mfe_url_override @@ -75,7 +77,8 @@ def _get_cloud_details(cloud: str = AzureEnvironments.ENV_DEFAULT): ) cloud = _get_default_cloud_name() try: - azure_environment = _environments[cloud] + all_clouds = _get_all_clouds() + azure_environment = all_clouds[cloud] module_logger.debug("Using the cloud configuration: '%s'.", azure_environment) except KeyError: raise Exception('Unknown cloud environment "{0}".'.format(cloud)) @@ -84,7 +87,7 @@ def _get_cloud_details(cloud: str = AzureEnvironments.ENV_DEFAULT): def _set_cloud(cloud: str = AzureEnvironments.ENV_DEFAULT): if cloud is not None: - if cloud not in _environments: + if cloud not in _get_all_clouds(): raise Exception('Unknown cloud environment supplied: "{0}".'.format(cloud)) else: cloud = _get_default_cloud_name() @@ -189,3 +192,130 @@ def _resource_to_scopes(resource): """ scope = resource + "/.default" return [scope] + + +_AZUREML_AUTH_CONFIG_DIR_ENV_NAME = 'AZUREML_AUTH_CONFIG_DIR' +def _get_config_dir(): + """Folder path for azureml-core to store authentication config""" + _AUTH_FOLDER_PATH = os.path.expanduser(os.path.join('~', '.azureml', "auth")) + if os.getenv(_AZUREML_AUTH_CONFIG_DIR_ENV_NAME, None): + return os.getenv(_AZUREML_AUTH_CONFIG_DIR_ENV_NAME, None) + else: + if sys.version_info > (3, 0): + os.makedirs(_AUTH_FOLDER_PATH, exist_ok=True) + else: + if not os.path.exists(_AUTH_FOLDER_PATH): + os.makedirs(_AUTH_FOLDER_PATH) + + return _AUTH_FOLDER_PATH + +GLOBAL_CONFIG_DIR = _get_config_dir() +CLOUD_CONFIG_FILE = os.path.join(GLOBAL_CONFIG_DIR, 'clouds.config') +DEFAULT_TIMEOUT = 30 +_DEFAULT_ARM_URL = "https://management.azure.com/metadata/endpoints?api-version=2019-05-01" +_ARM_METADATA_URL_ENV_NAME = "ARM_METADATA_URL" + +def _get_clouds_by_metadata_url(metadata_url, timeout=DEFAULT_TIMEOUT): + """Get all the clouds by the specified metadata url + + :return: list of the clouds + """ + try: + import requests + module_logger.debug('Start : Loading cloud metatdata from the url specified by {0}'.format(metadata_url)) + with requests.get(metadata_url, timeout=timeout) as meta_response: + arm_cloud_dict = meta_response.json() + cli_cloud_dict = _convert_arm_to_cli(arm_cloud_dict) + module_logger.debug('Finish : Loading cloud metatdata from the url specified by {0}'.format(metadata_url)) + return cli_cloud_dict + except Exception as ex: # pylint: disable=broad-except + module_logger.warning("Error: Azure ML was unable to load cloud metadata from the url specified by {0}. {1}. " + "This may be due to a misconfiguration of networking controls. Azure Machine Learning Python SDK " + "requires outbound access to Azure Resource Manager. Please contact your networking team to configure " + "outbound access to Azure Resource Manager on both Network Security Group and Firewall. " + "For more details on required configurations, see " + "https://docs.microsoft.com/azure/machine-learning/how-to-access-azureml-behind-firewall.".format( + metadata_url, ex)) + +def _convert_arm_to_cli(arm_cloud_metadata_dict): + cli_cloud_metadata_dict = {} + for cloud in arm_cloud_metadata_dict: + cli_cloud_metadata_dict[cloud['name']] = { + EndpointURLS.AZURE_PORTAL_ENDPOINT: cloud["portal"], + EndpointURLS.RESOURCE_MANAGER_ENDPOINT: cloud["resourceManager"], + EndpointURLS.ACTIVE_DIRECTORY_ENDPOINT: cloud["authentication"]["loginEndpoint"], + EndpointURLS.AML_RESOURCE_ID: cloud["resourceManager"], + EndpointURLS.STORAGE_ENDPOINT: cloud["suffixes"]["storage"] + } + return cli_cloud_metadata_dict + +def _get_cloud(cloud_name): + return next((data for name,data in _get_all_clouds().items() if name == cloud_name), None) + + +def _get_all_clouds(): + # Start with the hard coded list of clouds in this file + all_clouds = {} + all_clouds.update(_environments) + # Get configs from the config file + config = configparser.ConfigParser() + try: + config.read(CLOUD_CONFIG_FILE) + except configparser.MissingSectionHeaderError: + os.remove(CLOUD_CONFIG_FILE) + module_logger.warning("'%s' is in bad format and has been removed.", CLOUD_CONFIG_FILE) + for section in config.sections(): + all_clouds[section] = dict(config.items(section)) + # Now do the metadata URL + arm_url = os.environ[_ARM_METADATA_URL_ENV_NAME] if _ARM_METADATA_URL_ENV_NAME in os.environ else _DEFAULT_ARM_URL + all_clouds.update(_get_clouds_by_metadata_url(arm_url)) + # Send them all along with the hardcoded environments + return all_clouds + +class CloudNotRegisteredException(Exception): + def __init__(self, cloud_name): + super(CloudNotRegisteredException, self).__init__(cloud_name) + self.cloud_name = cloud_name + + def __str__(self): + return "The cloud '{}' is not registered.".format(self.cloud_name) + + +class CloudAlreadyRegisteredException(Exception): + def __init__(self, cloud_name): + super(CloudAlreadyRegisteredException, self).__init__(cloud_name) + self.cloud_name = cloud_name + + def __str__(self): + return "The cloud '{}' is already registered.".format(self.cloud_name) + +def _config_add_cloud(config, cloud, overwrite=False): + """ Add a cloud to a config object """ + try: + config.add_section(cloud["name"]) + except configparser.DuplicateSectionError: + if not overwrite: + raise CloudAlreadyRegisteredException(cloud["name"]) + for k,v in cloud.items(): + if k != "name" and v is not None: + config.set(cloud["name"], k, v) + +def _save_cloud(cloud, overwrite=False): + config = configparser.ConfigParser() + config.read(CLOUD_CONFIG_FILE) + _config_add_cloud(config, cloud, overwrite=overwrite) + if not os.path.isdir(GLOBAL_CONFIG_DIR): + os.makedirs(GLOBAL_CONFIG_DIR) + with open(CLOUD_CONFIG_FILE, 'w') as configfile: + config.write(configfile) + +def _add_cloud(cloud): + if _get_cloud(cloud["name"]): + raise CloudAlreadyRegisteredException(cloud["name"]) + _save_cloud(cloud) + + +def _update_cloud(cloud): + if not _get_cloud(cloud["name"]): + raise CloudNotRegisteredException(cloud["name"]) + _save_cloud(cloud, overwrite=True) diff --git a/sdk/ml/azure-ai-ml/azure/ai/ml/entities/_manage_clouds.py b/sdk/ml/azure-ai-ml/azure/ai/ml/entities/_manage_clouds.py new file mode 100644 index 000000000000..243e3830a40b --- /dev/null +++ b/sdk/ml/azure-ai-ml/azure/ai/ml/entities/_manage_clouds.py @@ -0,0 +1,7 @@ +from azure.ai.ml._azure_environments import (_add_cloud, _update_cloud) + +def add_cloud(cloud): + _add_cloud(cloud) + +def update_cloud(cloud): + _update_cloud(cloud) \ No newline at end of file From 69523c4d2e1b6f7fd9e50e2e894795b0cd14a66c Mon Sep 17 00:00:00 2001 From: Matt Brown Date: Tue, 31 Jan 2023 14:50:24 -0500 Subject: [PATCH 02/24] Removing some of the functionality, only ARM check remains --- sdk/ml/azure-ai-ml/azure/ai/ml/__init__.py | 6 - .../azure/ai/ml/_azure_environments.py | 104 +++--------------- .../azure/ai/ml/constants/_common.py | 4 + .../azure/ai/ml/entities/_manage_clouds.py | 7 -- 4 files changed, 22 insertions(+), 99 deletions(-) delete mode 100644 sdk/ml/azure-ai-ml/azure/ai/ml/entities/_manage_clouds.py diff --git a/sdk/ml/azure-ai-ml/azure/ai/ml/__init__.py b/sdk/ml/azure-ai-ml/azure/ai/ml/__init__.py index 803b1424fe10..9499e342c21a 100644 --- a/sdk/ml/azure-ai-ml/azure/ai/ml/__init__.py +++ b/sdk/ml/azure-ai-ml/azure/ai/ml/__init__.py @@ -32,10 +32,6 @@ load_workspace, load_workspace_connection, ) -from .entities._manage_clouds import ( - add_cloud, - update_cloud, -) module_logger = logging.getLogger(__name__) initialize_logger_info(module_logger, terminator="\n") @@ -67,8 +63,6 @@ "load_workspace", "load_registry", "load_workspace_connection", - "add_cloud", - "update_cloud", ] __version__ = VERSION diff --git a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py index 3c7c25e0f40d..b3a4a2709ccb 100644 --- a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py +++ b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py @@ -12,6 +12,7 @@ from azure.ai.ml._utils.utils import _get_mfe_url_override from azure.ai.ml.constants._common import AZUREML_CLOUD_ENV_NAME +from azure.ai.ml.constants._common import ArmConstants module_logger = logging.getLogger(__name__) @@ -193,29 +194,7 @@ def _resource_to_scopes(resource): scope = resource + "/.default" return [scope] - -_AZUREML_AUTH_CONFIG_DIR_ENV_NAME = 'AZUREML_AUTH_CONFIG_DIR' -def _get_config_dir(): - """Folder path for azureml-core to store authentication config""" - _AUTH_FOLDER_PATH = os.path.expanduser(os.path.join('~', '.azureml', "auth")) - if os.getenv(_AZUREML_AUTH_CONFIG_DIR_ENV_NAME, None): - return os.getenv(_AZUREML_AUTH_CONFIG_DIR_ENV_NAME, None) - else: - if sys.version_info > (3, 0): - os.makedirs(_AUTH_FOLDER_PATH, exist_ok=True) - else: - if not os.path.exists(_AUTH_FOLDER_PATH): - os.makedirs(_AUTH_FOLDER_PATH) - - return _AUTH_FOLDER_PATH - -GLOBAL_CONFIG_DIR = _get_config_dir() -CLOUD_CONFIG_FILE = os.path.join(GLOBAL_CONFIG_DIR, 'clouds.config') -DEFAULT_TIMEOUT = 30 -_DEFAULT_ARM_URL = "https://management.azure.com/metadata/endpoints?api-version=2019-05-01" -_ARM_METADATA_URL_ENV_NAME = "ARM_METADATA_URL" - -def _get_clouds_by_metadata_url(metadata_url, timeout=DEFAULT_TIMEOUT): +def _get_clouds_by_metadata_url(metadata_url, timeout=ArmConstants.DEFAULT_TIMEOUT): """Get all the clouds by the specified metadata url :return: list of the clouds @@ -236,86 +215,39 @@ def _get_clouds_by_metadata_url(metadata_url, timeout=DEFAULT_TIMEOUT): "For more details on required configurations, see " "https://docs.microsoft.com/azure/machine-learning/how-to-access-azureml-behind-firewall.".format( metadata_url, ex)) + return {} -def _convert_arm_to_cli(arm_cloud_metadata_dict): +def _convert_arm_to_cli(arm_cloud_metadata): cli_cloud_metadata_dict = {} - for cloud in arm_cloud_metadata_dict: - cli_cloud_metadata_dict[cloud['name']] = { - EndpointURLS.AZURE_PORTAL_ENDPOINT: cloud["portal"], - EndpointURLS.RESOURCE_MANAGER_ENDPOINT: cloud["resourceManager"], - EndpointURLS.ACTIVE_DIRECTORY_ENDPOINT: cloud["authentication"]["loginEndpoint"], - EndpointURLS.AML_RESOURCE_ID: cloud["resourceManager"], - EndpointURLS.STORAGE_ENDPOINT: cloud["suffixes"]["storage"] - } + if isinstance(arm_cloud_metadata, dict): + arm_cloud_metadata = [arm_cloud_metadata] + for cloud in arm_cloud_metadata: + try: + cli_cloud_metadata_dict[cloud['name']] = { + EndpointURLS.AZURE_PORTAL_ENDPOINT: cloud["portal"], + EndpointURLS.RESOURCE_MANAGER_ENDPOINT: cloud["resourceManager"], + EndpointURLS.ACTIVE_DIRECTORY_ENDPOINT: cloud["authentication"]["loginEndpoint"], + EndpointURLS.AML_RESOURCE_ID: cloud["resourceManager"], + EndpointURLS.STORAGE_ENDPOINT: cloud["suffixes"]["storage"] + } + except KeyError ex: + continue return cli_cloud_metadata_dict def _get_cloud(cloud_name): return next((data for name,data in _get_all_clouds().items() if name == cloud_name), None) - def _get_all_clouds(): # Start with the hard coded list of clouds in this file all_clouds = {} all_clouds.update(_environments) # Get configs from the config file config = configparser.ConfigParser() - try: - config.read(CLOUD_CONFIG_FILE) - except configparser.MissingSectionHeaderError: - os.remove(CLOUD_CONFIG_FILE) - module_logger.warning("'%s' is in bad format and has been removed.", CLOUD_CONFIG_FILE) for section in config.sections(): all_clouds[section] = dict(config.items(section)) # Now do the metadata URL - arm_url = os.environ[_ARM_METADATA_URL_ENV_NAME] if _ARM_METADATA_URL_ENV_NAME in os.environ else _DEFAULT_ARM_URL + arm_url = os.environ.get(ArmConstants.METADATA_URL_ENV_NAME,ArmConstants.DEFAULT_URL) all_clouds.update(_get_clouds_by_metadata_url(arm_url)) # Send them all along with the hardcoded environments return all_clouds -class CloudNotRegisteredException(Exception): - def __init__(self, cloud_name): - super(CloudNotRegisteredException, self).__init__(cloud_name) - self.cloud_name = cloud_name - - def __str__(self): - return "The cloud '{}' is not registered.".format(self.cloud_name) - - -class CloudAlreadyRegisteredException(Exception): - def __init__(self, cloud_name): - super(CloudAlreadyRegisteredException, self).__init__(cloud_name) - self.cloud_name = cloud_name - - def __str__(self): - return "The cloud '{}' is already registered.".format(self.cloud_name) - -def _config_add_cloud(config, cloud, overwrite=False): - """ Add a cloud to a config object """ - try: - config.add_section(cloud["name"]) - except configparser.DuplicateSectionError: - if not overwrite: - raise CloudAlreadyRegisteredException(cloud["name"]) - for k,v in cloud.items(): - if k != "name" and v is not None: - config.set(cloud["name"], k, v) - -def _save_cloud(cloud, overwrite=False): - config = configparser.ConfigParser() - config.read(CLOUD_CONFIG_FILE) - _config_add_cloud(config, cloud, overwrite=overwrite) - if not os.path.isdir(GLOBAL_CONFIG_DIR): - os.makedirs(GLOBAL_CONFIG_DIR) - with open(CLOUD_CONFIG_FILE, 'w') as configfile: - config.write(configfile) - -def _add_cloud(cloud): - if _get_cloud(cloud["name"]): - raise CloudAlreadyRegisteredException(cloud["name"]) - _save_cloud(cloud) - - -def _update_cloud(cloud): - if not _get_cloud(cloud["name"]): - raise CloudNotRegisteredException(cloud["name"]) - _save_cloud(cloud, overwrite=True) diff --git a/sdk/ml/azure-ai-ml/azure/ai/ml/constants/_common.py b/sdk/ml/azure-ai-ml/azure/ai/ml/constants/_common.py index 1832f8641c2e..fad5ba64e79c 100644 --- a/sdk/ml/azure-ai-ml/azure/ai/ml/constants/_common.py +++ b/sdk/ml/azure-ai-ml/azure/ai/ml/constants/_common.py @@ -273,6 +273,10 @@ class ArmConstants(object): AZURE_MGMT_KEYVAULT_API_VERSION = "2019-09-01" AZURE_MGMT_CONTAINER_REG_API_VERSION = "2019-05-01" + DEFAULT_TIMEOUT = 30 + DEFAULT_URL = "https://management.azure.com/metadata/endpoints?api-version=2019-05-01" + METADATA_URL_ENV_NAME = "ARM_CLOUD_METADATA_URL" + class HttpResponseStatusCode(object): NOT_FOUND = 404 diff --git a/sdk/ml/azure-ai-ml/azure/ai/ml/entities/_manage_clouds.py b/sdk/ml/azure-ai-ml/azure/ai/ml/entities/_manage_clouds.py deleted file mode 100644 index 243e3830a40b..000000000000 --- a/sdk/ml/azure-ai-ml/azure/ai/ml/entities/_manage_clouds.py +++ /dev/null @@ -1,7 +0,0 @@ -from azure.ai.ml._azure_environments import (_add_cloud, _update_cloud) - -def add_cloud(cloud): - _add_cloud(cloud) - -def update_cloud(cloud): - _update_cloud(cloud) \ No newline at end of file From 35d28ad9877b98068495f4821a0d583727d09086 Mon Sep 17 00:00:00 2001 From: Matt Brown Date: Wed, 1 Feb 2023 10:34:12 -0500 Subject: [PATCH 03/24] Adding unit test for new environments functionality --- .../azure/ai/ml/_azure_environments.py | 5 +-- .../unittests/test_cloud_environments.py | 45 ++++++++++++++++++- 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py index b3a4a2709ccb..7afdda22069d 100644 --- a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py +++ b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py @@ -230,13 +230,10 @@ def _convert_arm_to_cli(arm_cloud_metadata): EndpointURLS.AML_RESOURCE_ID: cloud["resourceManager"], EndpointURLS.STORAGE_ENDPOINT: cloud["suffixes"]["storage"] } - except KeyError ex: + except KeyError as ex: continue return cli_cloud_metadata_dict -def _get_cloud(cloud_name): - return next((data for name,data in _get_all_clouds().items() if name == cloud_name), None) - def _get_all_clouds(): # Start with the hard coded list of clouds in this file all_clouds = {} diff --git a/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py b/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py index be79f6fd68a1..25aaa255a822 100644 --- a/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py +++ b/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py @@ -2,6 +2,7 @@ import mock import pytest +from unittest.mock import patch from azure.ai.ml._azure_environments import ( AzureEnvironments, @@ -12,6 +13,8 @@ _get_registry_discovery_endpoint_from_metadata, _get_storage_endpoint_from_metadata, _set_cloud, + _get_all_clouds, + _get_clouds_by_metadata_url, ) from azure.ai.ml.constants._common import AZUREML_CLOUD_ENV_NAME @@ -88,4 +91,44 @@ def test_get_registry_endpoint_from_us_gov(self): cloud_environment = AzureEnvironments.ENV_US_GOVERNMENT _set_cloud(cloud_environment) base_url = _get_registry_discovery_endpoint_from_metadata(cloud_environment) - assert "https://usgovarizona.api.ml.azure.us/" in base_url \ No newline at end of file + assert "https://usgovarizona.api.ml.azure.us/" in base_url + + @patch('azure.ai.ml.requests.get') + def test_get_cloud_from_arm(self, mock_get): + mock_get.return_value.status_code = 201 + mock_get.return_value.json.return_value = [{ + "name": "TEST_ENV", + "portal": "testportal", + "resourceManager": "testresourcemanager", + "authentication": { + "loginEndpoint": "testdirectoryendpoint" + }, + "resourceManager": "testresourcemanager", + "suffixes": { + "storage": "teststorageendpoint" + } + }] + _set_cloud('TEST_ENV') + cloud_details = _get_cloud_information_from_metadata("TEST_ENV") + assert cloud_details.get("cloud") == "TEST_ENV" + + @patch('azure.ai.ml.requests.get') + def test_arm_misconfigured(self, mock_get): + mock_post.return_value.status_code = 201 + mock_post.return_value.json.return_value = [{ + "name": "TEST_ENV", + "portal": "testportal", + "resourceManager": "testresourcemanager", + "authentication": { + "loginEndpoint": "testdirectoryendpoint" + }, + "resourceManager": "testresourcemanager", + "suffixes": { + "storage": "teststorageendpoint" + } + }, + { + "name": "MISCONFIGURED" + }] + _set_cloud("MISCONFIGURED") + assert os.environ[AZUREML_CLOUD_ENV_NAME] != "MISCONFIGURED" From 69a581053b68217b8baa1caca059e231436be6b2 Mon Sep 17 00:00:00 2001 From: Ronald Shaw Date: Wed, 1 Feb 2023 20:34:11 -0500 Subject: [PATCH 04/24] fixed tests with mock --- .../azure/ai/ml/_azure_environments.py | 6 +- .../unittests/test_cloud_environments.py | 89 ++++++++++++------- 2 files changed, 60 insertions(+), 35 deletions(-) diff --git a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py index 7afdda22069d..d8629140ea6e 100644 --- a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py +++ b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py @@ -199,13 +199,15 @@ def _get_clouds_by_metadata_url(metadata_url, timeout=ArmConstants.DEFAULT_TIMEO :return: list of the clouds """ + import requests + try: - import requests module_logger.debug('Start : Loading cloud metatdata from the url specified by {0}'.format(metadata_url)) with requests.get(metadata_url, timeout=timeout) as meta_response: arm_cloud_dict = meta_response.json() cli_cloud_dict = _convert_arm_to_cli(arm_cloud_dict) - module_logger.debug('Finish : Loading cloud metatdata from the url specified by {0}'.format(metadata_url)) + print("cli_cloud_dict: ", cli_cloud_dict) + module_logger.debug('Finish : Loading cloud metadata from the url specified by {0}'.format(metadata_url)) return cli_cloud_dict except Exception as ex: # pylint: disable=broad-except module_logger.warning("Error: Azure ML was unable to load cloud metadata from the url specified by {0}. {1}. " diff --git a/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py b/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py index 25aaa255a822..4cae1188d8a7 100644 --- a/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py +++ b/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py @@ -1,8 +1,8 @@ import os - import mock import pytest -from unittest.mock import patch +from mock import MagicMock, patch +import requests from azure.ai.ml._azure_environments import ( AzureEnvironments, @@ -16,12 +16,16 @@ _get_all_clouds, _get_clouds_by_metadata_url, ) -from azure.ai.ml.constants._common import AZUREML_CLOUD_ENV_NAME +from azure.ai.ml.constants._common import ArmConstants, AZUREML_CLOUD_ENV_NAME + + + @pytest.mark.unittest @pytest.mark.core_sdk_test class TestCloudEnvironments: + @mock.patch.dict(os.environ, {AZUREML_CLOUD_ENV_NAME: AzureEnvironments.ENV_DEFAULT}, clear=True) def test_set_valid_cloud_details_china(self): cloud_environment = AzureEnvironments.ENV_CHINA @@ -93,42 +97,61 @@ def test_get_registry_endpoint_from_us_gov(self): base_url = _get_registry_discovery_endpoint_from_metadata(cloud_environment) assert "https://usgovarizona.api.ml.azure.us/" in base_url - @patch('azure.ai.ml.requests.get') - def test_get_cloud_from_arm(self, mock_get): - mock_get.return_value.status_code = 201 - mock_get.return_value.json.return_value = [{ - "name": "TEST_ENV", - "portal": "testportal", - "resourceManager": "testresourcemanager", - "authentication": { - "loginEndpoint": "testdirectoryendpoint" + + + + def mocked_requests_get(*args, **kwargs): + print("in the function") + class MockResponse: + def __init__(self, json_data): + self.json_data = json_data + self.status_code = 201 + def __enter__(self): + return self; + + def __exit__(self, exc_type, exc_value, traceback): + return + + def json(self): + return self.json_data + + print("past the class") + json_data = [ + { + "name": "TEST_ENV", + "portal": "testportal", + "resourceManager": "testresourcemanager", + "authentication": { + "loginEndpoint": "testdirectoryendpoint" + }, + "resourceManager": "testresourcemanager", + "suffixes": { + "storage": "teststorageendpoint" + } }, - "resourceManager": "testresourcemanager", - "suffixes": { - "storage": "teststorageendpoint" + { + "name": "MISCONFIGURED" } - }] + ] + print("past the json") + response = MockResponse(json_data) + return response + + + @mock.patch('requests.get', side_effect=mocked_requests_get) + def test_get_cloud_from_arm(self, mock_get): + _set_cloud('TEST_ENV') cloud_details = _get_cloud_information_from_metadata("TEST_ENV") assert cloud_details.get("cloud") == "TEST_ENV" + # cloud_details = _get_clouds_by_metadata_url(ArmConstants.DEFAULT_URL) + # for key,values in cloud_details.items(): + # assert key == "TEST_ENV" - @patch('azure.ai.ml.requests.get') + + @mock.patch('requests.get', side_effect=mocked_requests_get) def test_arm_misconfigured(self, mock_get): - mock_post.return_value.status_code = 201 - mock_post.return_value.json.return_value = [{ - "name": "TEST_ENV", - "portal": "testportal", - "resourceManager": "testresourcemanager", - "authentication": { - "loginEndpoint": "testdirectoryendpoint" - }, - "resourceManager": "testresourcemanager", - "suffixes": { - "storage": "teststorageendpoint" - } - }, - { - "name": "MISCONFIGURED" - }] + _set_cloud("MISCONFIGURED") assert os.environ[AZUREML_CLOUD_ENV_NAME] != "MISCONFIGURED" + From 0a461e83a1fc17e3d9cfe7eb08187d7c17c1b0a9 Mon Sep 17 00:00:00 2001 From: Ronald Shaw Date: Thu, 2 Feb 2023 10:07:51 -0500 Subject: [PATCH 05/24] removed print statement --- sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py | 1 - 1 file changed, 1 deletion(-) diff --git a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py index d8629140ea6e..80f190263e11 100644 --- a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py +++ b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py @@ -206,7 +206,6 @@ def _get_clouds_by_metadata_url(metadata_url, timeout=ArmConstants.DEFAULT_TIMEO with requests.get(metadata_url, timeout=timeout) as meta_response: arm_cloud_dict = meta_response.json() cli_cloud_dict = _convert_arm_to_cli(arm_cloud_dict) - print("cli_cloud_dict: ", cli_cloud_dict) module_logger.debug('Finish : Loading cloud metadata from the url specified by {0}'.format(metadata_url)) return cli_cloud_dict except Exception as ex: # pylint: disable=broad-except From 0dcc7b280bbd925ade811ca2b7d6aad06dbc6817 Mon Sep 17 00:00:00 2001 From: Ronald Shaw Date: Thu, 2 Feb 2023 10:12:49 -0500 Subject: [PATCH 06/24] removed commented code --- .../internal_utils/unittests/test_cloud_environments.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py b/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py index 4cae1188d8a7..299c2b65a8ac 100644 --- a/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py +++ b/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py @@ -144,10 +144,7 @@ def test_get_cloud_from_arm(self, mock_get): _set_cloud('TEST_ENV') cloud_details = _get_cloud_information_from_metadata("TEST_ENV") assert cloud_details.get("cloud") == "TEST_ENV" - # cloud_details = _get_clouds_by_metadata_url(ArmConstants.DEFAULT_URL) - # for key,values in cloud_details.items(): - # assert key == "TEST_ENV" - + @mock.patch('requests.get', side_effect=mocked_requests_get) def test_arm_misconfigured(self, mock_get): From 4210a4eabbdce592eeeb4fea6e25eb1cea43c029 Mon Sep 17 00:00:00 2001 From: Ronald Shaw Date: Thu, 2 Feb 2023 11:15:51 -0500 Subject: [PATCH 07/24] removed print statements (oops) --- .../tests/internal_utils/unittests/test_cloud_environments.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py b/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py index 299c2b65a8ac..2b7e9e0da48e 100644 --- a/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py +++ b/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py @@ -101,7 +101,6 @@ def test_get_registry_endpoint_from_us_gov(self): def mocked_requests_get(*args, **kwargs): - print("in the function") class MockResponse: def __init__(self, json_data): self.json_data = json_data @@ -115,7 +114,6 @@ def __exit__(self, exc_type, exc_value, traceback): def json(self): return self.json_data - print("past the class") json_data = [ { "name": "TEST_ENV", @@ -133,7 +131,6 @@ def json(self): "name": "MISCONFIGURED" } ] - print("past the json") response = MockResponse(json_data) return response From 53c7db02eacf861712368e9eb125dcec745cc0ab Mon Sep 17 00:00:00 2001 From: Ronald Shaw Date: Thu, 2 Feb 2023 12:06:18 -0500 Subject: [PATCH 08/24] fixed tests and removed comments --- .../azure-ai-ml/azure/ai/ml/_azure_environments.py | 13 ++++++------- .../unittests/test_cloud_environments.py | 6 ++---- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py index 80f190263e11..b947b9bf0562 100644 --- a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py +++ b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py @@ -230,22 +230,21 @@ def _convert_arm_to_cli(arm_cloud_metadata): EndpointURLS.ACTIVE_DIRECTORY_ENDPOINT: cloud["authentication"]["loginEndpoint"], EndpointURLS.AML_RESOURCE_ID: cloud["resourceManager"], EndpointURLS.STORAGE_ENDPOINT: cloud["suffixes"]["storage"] + } except KeyError as ex: continue return cli_cloud_metadata_dict def _get_all_clouds(): - # Start with the hard coded list of clouds in this file + # Start with the metadata URL all_clouds = {} - all_clouds.update(_environments) - # Get configs from the config file - config = configparser.ConfigParser() - for section in config.sections(): - all_clouds[section] = dict(config.items(section)) - # Now do the metadata URL arm_url = os.environ.get(ArmConstants.METADATA_URL_ENV_NAME,ArmConstants.DEFAULT_URL) all_clouds.update(_get_clouds_by_metadata_url(arm_url)) + + # Now the hard coded list of clouds in this file + all_clouds.update(_environments) + # Send them all along with the hardcoded environments return all_clouds diff --git a/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py b/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py index 2b7e9e0da48e..ba5bce5433eb 100644 --- a/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py +++ b/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py @@ -145,7 +145,5 @@ def test_get_cloud_from_arm(self, mock_get): @mock.patch('requests.get', side_effect=mocked_requests_get) def test_arm_misconfigured(self, mock_get): - - _set_cloud("MISCONFIGURED") - assert os.environ[AZUREML_CLOUD_ENV_NAME] != "MISCONFIGURED" - + with pytest.raises(Exception) as e_info: + _set_cloud("MISCONFIGURED") From 7fe48d2c14464a84145511053e884ac27824c3ec Mon Sep 17 00:00:00 2001 From: Matt Brown Date: Thu, 2 Feb 2023 14:44:34 -0500 Subject: [PATCH 09/24] Fixing a testing bug --- .../unittests/test_cloud_environments.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py b/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py index ba5bce5433eb..a7f4e7d78e7b 100644 --- a/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py +++ b/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py @@ -100,7 +100,7 @@ def test_get_registry_endpoint_from_us_gov(self): - def mocked_requests_get(*args, **kwargs): + def mock_arm_response(*args, **kwargs): class MockResponse: def __init__(self, json_data): self.json_data = json_data @@ -133,17 +133,16 @@ def json(self): ] response = MockResponse(json_data) return response - - @mock.patch('requests.get', side_effect=mocked_requests_get) + @mock.patch.dict(os.environ, {}, clear=True) + @mock.patch('requests.get', side_effect=mock_arm_response) def test_get_cloud_from_arm(self, mock_get): - _set_cloud('TEST_ENV') cloud_details = _get_cloud_information_from_metadata("TEST_ENV") assert cloud_details.get("cloud") == "TEST_ENV" - - @mock.patch('requests.get', side_effect=mocked_requests_get) + @mock.patch.dict(os.environ, {}, clear=True) + @mock.patch('requests.get', side_effect=mock_arm_response) def test_arm_misconfigured(self, mock_get): with pytest.raises(Exception) as e_info: _set_cloud("MISCONFIGURED") From f3939b2ab7acce0080cbea2bf6a7e3e255b0866d Mon Sep 17 00:00:00 2001 From: Matt Brown Date: Thu, 2 Feb 2023 15:43:47 -0500 Subject: [PATCH 10/24] Fixing a misspelled word --- sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py index b947b9bf0562..18e2a5b1f1e0 100644 --- a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py +++ b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py @@ -202,7 +202,7 @@ def _get_clouds_by_metadata_url(metadata_url, timeout=ArmConstants.DEFAULT_TIMEO import requests try: - module_logger.debug('Start : Loading cloud metatdata from the url specified by {0}'.format(metadata_url)) + module_logger.debug('Start : Loading cloud metadata from the url specified by {0}'.format(metadata_url)) with requests.get(metadata_url, timeout=timeout) as meta_response: arm_cloud_dict = meta_response.json() cli_cloud_dict = _convert_arm_to_cli(arm_cloud_dict) From 215cf30a302d5e9f82f169ed5c940b84f3d890a5 Mon Sep 17 00:00:00 2001 From: Matt Brown Date: Thu, 2 Feb 2023 18:22:25 -0500 Subject: [PATCH 11/24] Changing how we reach out to ARM, als fixing some pylint --- .../azure/ai/ml/_azure_environments.py | 47 ++++++++----------- .../unittests/test_cloud_environments.py | 2 - 2 files changed, 20 insertions(+), 29 deletions(-) diff --git a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py index 18e2a5b1f1e0..f2ee03c99f9d 100644 --- a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py +++ b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py @@ -59,6 +59,16 @@ class EndpointURLS: # pylint: disable=too-few-public-methods,no-init }, } +def _get_cloud(cloud: str): + if cloud in _environments: + return _environments[cloud] + else: + arm_url = os.environ.get(ArmConstants.METADATA_URL_ENV_NAME,ArmConstants.DEFAULT_URL) + arm_clouds = _get_clouds_by_metadata_url(arm_url) + try: + return arm_clouds[cloud] + except KeyError: + raise Exception('Unknown cloud environment "{0}".'.format(cloud)) def _get_default_cloud_name(): """Return AzureCloud as the default cloud.""" @@ -77,18 +87,14 @@ def _get_cloud_details(cloud: str = AzureEnvironments.ENV_DEFAULT): AzureEnvironments.ENV_DEFAULT, ) cloud = _get_default_cloud_name() - try: - all_clouds = _get_all_clouds() - azure_environment = all_clouds[cloud] - module_logger.debug("Using the cloud configuration: '%s'.", azure_environment) - except KeyError: - raise Exception('Unknown cloud environment "{0}".'.format(cloud)) - return azure_environment + return _get_cloud(cloud) def _set_cloud(cloud: str = AzureEnvironments.ENV_DEFAULT): if cloud is not None: - if cloud not in _get_all_clouds(): + try: + _get_cloud(cloud) + except Exception: raise Exception('Unknown cloud environment supplied: "{0}".'.format(cloud)) else: cloud = _get_default_cloud_name() @@ -210,12 +216,12 @@ def _get_clouds_by_metadata_url(metadata_url, timeout=ArmConstants.DEFAULT_TIMEO return cli_cloud_dict except Exception as ex: # pylint: disable=broad-except module_logger.warning("Error: Azure ML was unable to load cloud metadata from the url specified by {0}. {1}. " - "This may be due to a misconfiguration of networking controls. Azure Machine Learning Python SDK " - "requires outbound access to Azure Resource Manager. Please contact your networking team to configure " - "outbound access to Azure Resource Manager on both Network Security Group and Firewall. " - "For more details on required configurations, see " - "https://docs.microsoft.com/azure/machine-learning/how-to-access-azureml-behind-firewall.".format( - metadata_url, ex)) + "This may be due to a misconfiguration of networking controls. Azure Machine Learning Python " + "SDK requires outbound access to Azure Resource Manager. Please contact your networking team " + "to configure outbound access to Azure Resource Manager on both Network Security Group and " + "Firewall. For more details on required configurations, see " + "https://docs.microsoft.com/azure/machine-learning/how-to-access-azureml-behind-firewall." + .format(metadata_url, ex)) return {} def _convert_arm_to_cli(arm_cloud_metadata): @@ -235,16 +241,3 @@ def _convert_arm_to_cli(arm_cloud_metadata): except KeyError as ex: continue return cli_cloud_metadata_dict - -def _get_all_clouds(): - # Start with the metadata URL - all_clouds = {} - arm_url = os.environ.get(ArmConstants.METADATA_URL_ENV_NAME,ArmConstants.DEFAULT_URL) - all_clouds.update(_get_clouds_by_metadata_url(arm_url)) - - # Now the hard coded list of clouds in this file - all_clouds.update(_environments) - - # Send them all along with the hardcoded environments - return all_clouds - diff --git a/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py b/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py index a7f4e7d78e7b..86b359e33f50 100644 --- a/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py +++ b/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py @@ -13,8 +13,6 @@ _get_registry_discovery_endpoint_from_metadata, _get_storage_endpoint_from_metadata, _set_cloud, - _get_all_clouds, - _get_clouds_by_metadata_url, ) from azure.ai.ml.constants._common import ArmConstants, AZUREML_CLOUD_ENV_NAME From 571b4bb4c4f6b8341d81b9a42161956fe6d67476 Mon Sep 17 00:00:00 2001 From: Matt Brown Date: Thu, 2 Feb 2023 20:55:21 -0500 Subject: [PATCH 12/24] Fixing more lint errors --- .../azure/ai/ml/_azure_environments.py | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py index f2ee03c99f9d..33ae82410d23 100644 --- a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py +++ b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py @@ -4,10 +4,8 @@ """Metadata to interact with different Azure clouds.""" -import configparser import logging import os -import sys from typing import Dict, Optional from azure.ai.ml._utils.utils import _get_mfe_url_override @@ -62,13 +60,12 @@ class EndpointURLS: # pylint: disable=too-few-public-methods,no-init def _get_cloud(cloud: str): if cloud in _environments: return _environments[cloud] - else: - arm_url = os.environ.get(ArmConstants.METADATA_URL_ENV_NAME,ArmConstants.DEFAULT_URL) - arm_clouds = _get_clouds_by_metadata_url(arm_url) - try: - return arm_clouds[cloud] - except KeyError: - raise Exception('Unknown cloud environment "{0}".'.format(cloud)) + arm_url = os.environ.get(ArmConstants.METADATA_URL_ENV_NAME,ArmConstants.DEFAULT_URL) + arm_clouds = _get_clouds_by_metadata_url(arm_url) + try: + return arm_clouds[cloud] + except KeyError: + raise Exception('Unknown cloud environment "{0}".'.format(cloud)) def _get_default_cloud_name(): """Return AzureCloud as the default cloud.""" @@ -92,7 +89,7 @@ def _get_cloud_details(cloud: str = AzureEnvironments.ENV_DEFAULT): def _set_cloud(cloud: str = AzureEnvironments.ENV_DEFAULT): if cloud is not None: - try: + try: _get_cloud(cloud) except Exception: raise Exception('Unknown cloud environment supplied: "{0}".'.format(cloud)) @@ -215,13 +212,13 @@ def _get_clouds_by_metadata_url(metadata_url, timeout=ArmConstants.DEFAULT_TIMEO module_logger.debug('Finish : Loading cloud metadata from the url specified by {0}'.format(metadata_url)) return cli_cloud_dict except Exception as ex: # pylint: disable=broad-except - module_logger.warning("Error: Azure ML was unable to load cloud metadata from the url specified by {0}. {1}. " + module_logger.warning("Error: Azure ML was unable to load cloud metadata from the url specified by %s. %s. " "This may be due to a misconfiguration of networking controls. Azure Machine Learning Python " "SDK requires outbound access to Azure Resource Manager. Please contact your networking team " "to configure outbound access to Azure Resource Manager on both Network Security Group and " "Firewall. For more details on required configurations, see " - "https://docs.microsoft.com/azure/machine-learning/how-to-access-azureml-behind-firewall." - .format(metadata_url, ex)) + "https://docs.microsoft.com/azure/machine-learning/how-to-access-azureml-behind-firewall.", + metadata_url, ex) return {} def _convert_arm_to_cli(arm_cloud_metadata): @@ -238,6 +235,6 @@ def _convert_arm_to_cli(arm_cloud_metadata): EndpointURLS.STORAGE_ENDPOINT: cloud["suffixes"]["storage"] } - except KeyError as ex: + except KeyError: continue return cli_cloud_metadata_dict From f04d3999d94d0409f707383293fb99ac97564e56 Mon Sep 17 00:00:00 2001 From: Matt Brown Date: Thu, 2 Feb 2023 21:48:41 -0500 Subject: [PATCH 13/24] Fixing more lint errors --- sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py index 33ae82410d23..196131d4be63 100644 --- a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py +++ b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py @@ -205,11 +205,11 @@ def _get_clouds_by_metadata_url(metadata_url, timeout=ArmConstants.DEFAULT_TIMEO import requests try: - module_logger.debug('Start : Loading cloud metadata from the url specified by {0}'.format(metadata_url)) + module_logger.debug('Start : Loading cloud metadata from the url specified by %s', metadata_url) with requests.get(metadata_url, timeout=timeout) as meta_response: arm_cloud_dict = meta_response.json() cli_cloud_dict = _convert_arm_to_cli(arm_cloud_dict) - module_logger.debug('Finish : Loading cloud metadata from the url specified by {0}'.format(metadata_url)) + module_logger.debug('Finish : Loading cloud metadata from the url specified by %s', metadata_url) return cli_cloud_dict except Exception as ex: # pylint: disable=broad-except module_logger.warning("Error: Azure ML was unable to load cloud metadata from the url specified by %s. %s. " From 2254a5b99013924b9fa7cebb101f0c5aa711b13a Mon Sep 17 00:00:00 2001 From: Ronald Shaw Date: Fri, 3 Feb 2023 09:48:11 -0500 Subject: [PATCH 14/24] updated code per suggestions in PR --- sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py index 196131d4be63..bf182eaa24e6 100644 --- a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py +++ b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py @@ -63,7 +63,9 @@ def _get_cloud(cloud: str): arm_url = os.environ.get(ArmConstants.METADATA_URL_ENV_NAME,ArmConstants.DEFAULT_URL) arm_clouds = _get_clouds_by_metadata_url(arm_url) try: - return arm_clouds[cloud] + new_cloud = arm_clouds[cloud] + _environments.update(new_cloud) + return new_cloud except KeyError: raise Exception('Unknown cloud environment "{0}".'.format(cloud)) @@ -235,6 +237,7 @@ def _convert_arm_to_cli(arm_cloud_metadata): EndpointURLS.STORAGE_ENDPOINT: cloud["suffixes"]["storage"] } - except KeyError: + except KeyError as ex: + module_logger.warning("Property on cloud ot found in arm cloud metadata: {}".format(ex)) continue return cli_cloud_metadata_dict From 22da3387adc9647ef952de20f314f284915ad51f Mon Sep 17 00:00:00 2001 From: Ronald Shaw Date: Fri, 3 Feb 2023 10:29:53 -0500 Subject: [PATCH 15/24] fixed typo in warning --- sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py index bf182eaa24e6..1924e22704eb 100644 --- a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py +++ b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py @@ -238,6 +238,6 @@ def _convert_arm_to_cli(arm_cloud_metadata): } except KeyError as ex: - module_logger.warning("Property on cloud ot found in arm cloud metadata: {}".format(ex)) + module_logger.warning("Property on cloud not found in arm cloud metadata: {}".format(ex)) continue return cli_cloud_metadata_dict From 88586b12beaa9c65032fcf7f53086d775c1dfb99 Mon Sep 17 00:00:00 2001 From: Ronald Shaw Date: Fri, 3 Feb 2023 13:55:35 -0500 Subject: [PATCH 16/24] added registry_endpoint to metadata url, also added tests for making sure all endpointurls are registered --- .../azure/ai/ml/_azure_environments.py | 11 ++++++---- .../unittests/test_cloud_environments.py | 21 +++++++++++++++---- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py index 1924e22704eb..bc7a606b2a01 100644 --- a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py +++ b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py @@ -229,13 +229,16 @@ def _convert_arm_to_cli(arm_cloud_metadata): arm_cloud_metadata = [arm_cloud_metadata] for cloud in arm_cloud_metadata: try: - cli_cloud_metadata_dict[cloud['name']] = { + portal_endpoint = cloud["portal"] + cloud_suffix = portal_endpoint.split('.')[2] + cloud_name = cloud['name'] + cli_cloud_metadata_dict[cloud_name] = { EndpointURLS.AZURE_PORTAL_ENDPOINT: cloud["portal"], EndpointURLS.RESOURCE_MANAGER_ENDPOINT: cloud["resourceManager"], EndpointURLS.ACTIVE_DIRECTORY_ENDPOINT: cloud["authentication"]["loginEndpoint"], - EndpointURLS.AML_RESOURCE_ID: cloud["resourceManager"], - EndpointURLS.STORAGE_ENDPOINT: cloud["suffixes"]["storage"] - + EndpointURLS.AML_RESOURCE_ID: "https://ml.azure.{}".format(cloud_suffix), + EndpointURLS.STORAGE_ENDPOINT: cloud["suffixes"]["storage"], + EndpointURLS.REGISTRY_DISCOVERY_ENDPOINT: "https://{}west.api.azureml.{}/".format(cloud_name.lower(), cloud_suffix) } except KeyError as ex: module_logger.warning("Property on cloud not found in arm cloud metadata: {}".format(ex)) diff --git a/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py b/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py index 86b359e33f50..74499999fafe 100644 --- a/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py +++ b/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py @@ -6,8 +6,10 @@ from azure.ai.ml._azure_environments import ( AzureEnvironments, + EndpointURLS, _get_azure_portal_id_from_metadata, _get_base_url_from_metadata, + _get_cloud_details, _get_cloud_information_from_metadata, _get_default_cloud_name, _get_registry_discovery_endpoint_from_metadata, @@ -115,12 +117,11 @@ def json(self): json_data = [ { "name": "TEST_ENV", - "portal": "testportal", - "resourceManager": "testresourcemanager", + "portal": "testportal.azure.com", + "resourceManager": "testresourcemanager.azure.com", "authentication": { - "loginEndpoint": "testdirectoryendpoint" + "loginEndpoint": "testdirectoryendpoint.azure.com" }, - "resourceManager": "testresourcemanager", "suffixes": { "storage": "teststorageendpoint" } @@ -138,6 +139,18 @@ def test_get_cloud_from_arm(self, mock_get): _set_cloud('TEST_ENV') cloud_details = _get_cloud_information_from_metadata("TEST_ENV") assert cloud_details.get("cloud") == "TEST_ENV" + + @mock.patch.dict(os.environ, {}, clear=True) + @mock.patch('requests.get', side_effect=mock_arm_response) + def test_all_endpointurls_used(self, mock_get): + cloud_details = _get_cloud_details("TEST_ENV") + endpoint_urls = [a for a in dir(EndpointURLS) if not a.startswith('__')] + for url in endpoint_urls: + try: + cloud_details[EndpointURLS.__dict__[url]] + except: + assert False, "Url not found: {}".format(EndpointURLS.__dict__[url]) + assert True @mock.patch.dict(os.environ, {}, clear=True) @mock.patch('requests.get', side_effect=mock_arm_response) From badc3a3d3374989283e6944387cbbb0b93d9371a Mon Sep 17 00:00:00 2001 From: Ronald Shaw Date: Fri, 3 Feb 2023 15:33:24 -0500 Subject: [PATCH 17/24] updated how the registry discovery endpoint is created. Uses a default region but region can be updated with environment variable --- .../azure/ai/ml/_azure_environments.py | 12 +++++++++--- .../azure/ai/ml/constants/_common.py | 2 ++ .../unittests/test_cloud_environments.py | 17 +++++++++++++++++ 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py index bc7a606b2a01..06898c1233f9 100644 --- a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py +++ b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py @@ -227,10 +227,12 @@ def _convert_arm_to_cli(arm_cloud_metadata): cli_cloud_metadata_dict = {} if isinstance(arm_cloud_metadata, dict): arm_cloud_metadata = [arm_cloud_metadata] + for cloud in arm_cloud_metadata: try: + registry_discovery_region = os.environ.get(ArmConstants.REGISTRY_DISCOVERY_REGION_ENV_NAME, ArmConstants.REGISTRY_DISCOVERY_DEFAULT_REGION) portal_endpoint = cloud["portal"] - cloud_suffix = portal_endpoint.split('.')[2] + cloud_suffix = ".".join(portal_endpoint.split('.')[2:]).replace("/", "") cloud_name = cloud['name'] cli_cloud_metadata_dict[cloud_name] = { EndpointURLS.AZURE_PORTAL_ENDPOINT: cloud["portal"], @@ -238,9 +240,13 @@ def _convert_arm_to_cli(arm_cloud_metadata): EndpointURLS.ACTIVE_DIRECTORY_ENDPOINT: cloud["authentication"]["loginEndpoint"], EndpointURLS.AML_RESOURCE_ID: "https://ml.azure.{}".format(cloud_suffix), EndpointURLS.STORAGE_ENDPOINT: cloud["suffixes"]["storage"], - EndpointURLS.REGISTRY_DISCOVERY_ENDPOINT: "https://{}west.api.azureml.{}/".format(cloud_name.lower(), cloud_suffix) + EndpointURLS.REGISTRY_DISCOVERY_ENDPOINT: "https://{}{}.api.azureml.{}/".format( + cloud_name.lower(), + registry_discovery_region, + cloud_suffix + ) } except KeyError as ex: - module_logger.warning("Property on cloud not found in arm cloud metadata: {}".format(ex)) + module_logger.warning("Property on cloud not found in arm cloud metadata: %s", ex) continue return cli_cloud_metadata_dict diff --git a/sdk/ml/azure-ai-ml/azure/ai/ml/constants/_common.py b/sdk/ml/azure-ai-ml/azure/ai/ml/constants/_common.py index fad5ba64e79c..6d7b23ce5a0b 100644 --- a/sdk/ml/azure-ai-ml/azure/ai/ml/constants/_common.py +++ b/sdk/ml/azure-ai-ml/azure/ai/ml/constants/_common.py @@ -276,6 +276,8 @@ class ArmConstants(object): DEFAULT_TIMEOUT = 30 DEFAULT_URL = "https://management.azure.com/metadata/endpoints?api-version=2019-05-01" METADATA_URL_ENV_NAME = "ARM_CLOUD_METADATA_URL" + REGISTRY_DISCOVERY_DEFAULT_REGION = "west" + REGISTRY_DISCOVERY_REGION_ENV_NAME = "REGISTRY_DISCOVERY_ENDPOINT_REGION" class HttpResponseStatusCode(object): diff --git a/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py b/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py index 74499999fafe..7e3c123b7773 100644 --- a/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py +++ b/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py @@ -126,6 +126,17 @@ def json(self): "storage": "teststorageendpoint" } }, + { + "name": "TEST_ENV2", + "portal": "testportal.azure.windows.net", + "resourceManager": "testresourcemanager.azure.com", + "authentication": { + "loginEndpoint": "testdirectoryendpoint.azure.com" + }, + "suffixes": { + "storage": "teststorageendpoint" + } + }, { "name": "MISCONFIGURED" } @@ -151,6 +162,12 @@ def test_all_endpointurls_used(self, mock_get): except: assert False, "Url not found: {}".format(EndpointURLS.__dict__[url]) assert True + + @mock.patch.dict(os.environ, {}, clear=True) + @mock.patch('requests.get', side_effect=mock_arm_response) + def test_metadata_registry_endpoint(self, mock_get): + cloud_details = _get_cloud_details("TEST_ENV2") + assert cloud_details.get(EndpointURLS.REGISTRY_DISCOVERY_ENDPOINT) == "https://test_env2west.api.azureml.windows.net/" @mock.patch.dict(os.environ, {}, clear=True) @mock.patch('requests.get', side_effect=mock_arm_response) From 390eeed39d5b28878139f0b4ec8f033ba6e5f5b8 Mon Sep 17 00:00:00 2001 From: Ronald Shaw Date: Fri, 3 Feb 2023 16:13:23 -0500 Subject: [PATCH 18/24] fixed linting errors --- sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py index 06898c1233f9..b586e1e20091 100644 --- a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py +++ b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py @@ -227,10 +227,13 @@ def _convert_arm_to_cli(arm_cloud_metadata): cli_cloud_metadata_dict = {} if isinstance(arm_cloud_metadata, dict): arm_cloud_metadata = [arm_cloud_metadata] - + for cloud in arm_cloud_metadata: try: - registry_discovery_region = os.environ.get(ArmConstants.REGISTRY_DISCOVERY_REGION_ENV_NAME, ArmConstants.REGISTRY_DISCOVERY_DEFAULT_REGION) + registry_discovery_region = os.environ.get( + ArmConstants.REGISTRY_DISCOVERY_REGION_ENV_NAME, + ArmConstants.REGISTRY_DISCOVERY_DEFAULT_REGION + ) portal_endpoint = cloud["portal"] cloud_suffix = ".".join(portal_endpoint.split('.')[2:]).replace("/", "") cloud_name = cloud['name'] From 9ff796c4062263a2bb4d4345aea0848bedd5b718 Mon Sep 17 00:00:00 2001 From: Ronald Shaw Date: Fri, 3 Feb 2023 17:55:14 -0500 Subject: [PATCH 19/24] moved discovery url logic around to make sure it's not overwriting public regions --- .../azure/ai/ml/_azure_environments.py | 33 +++++++++++++------ .../azure/ai/ml/constants/_common.py | 1 + .../unittests/test_cloud_environments.py | 2 +- 3 files changed, 25 insertions(+), 11 deletions(-) diff --git a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py index b586e1e20091..7ebcd537bf7a 100644 --- a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py +++ b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py @@ -199,6 +199,26 @@ def _resource_to_scopes(resource): scope = resource + "/.default" return [scope] +def _get_registry_discovery_url(cloud, cloud_suffix): + """Get the registry discovery url + :return: string of discovery url + """ + cloud_name = cloud["name"] + if cloud_name in _environments: + return _environments[cloud_name].registry_url + elif ArmConstants.REGISTRY_ENV_URL in os.environ: + return os.environ[ArmConstants.REGISTRY_ENV_URL] + else: + registry_discovery_region = os.environ.get( + ArmConstants.REGISTRY_DISCOVERY_REGION_ENV_NAME, + ArmConstants.REGISTRY_DISCOVERY_DEFAULT_REGION + ) + return "https://{}{}.api.azureml.{}/".format( + cloud_name.lower(), + registry_discovery_region, + cloud_suffix + ) + def _get_clouds_by_metadata_url(metadata_url, timeout=ArmConstants.DEFAULT_TIMEOUT): """Get all the clouds by the specified metadata url @@ -230,24 +250,17 @@ def _convert_arm_to_cli(arm_cloud_metadata): for cloud in arm_cloud_metadata: try: - registry_discovery_region = os.environ.get( - ArmConstants.REGISTRY_DISCOVERY_REGION_ENV_NAME, - ArmConstants.REGISTRY_DISCOVERY_DEFAULT_REGION - ) + cloud_name = cloud["name"] portal_endpoint = cloud["portal"] cloud_suffix = ".".join(portal_endpoint.split('.')[2:]).replace("/", "") - cloud_name = cloud['name'] + registry_discovery_url = _get_registry_discovery_url(cloud, cloud_suffix) cli_cloud_metadata_dict[cloud_name] = { EndpointURLS.AZURE_PORTAL_ENDPOINT: cloud["portal"], EndpointURLS.RESOURCE_MANAGER_ENDPOINT: cloud["resourceManager"], EndpointURLS.ACTIVE_DIRECTORY_ENDPOINT: cloud["authentication"]["loginEndpoint"], EndpointURLS.AML_RESOURCE_ID: "https://ml.azure.{}".format(cloud_suffix), EndpointURLS.STORAGE_ENDPOINT: cloud["suffixes"]["storage"], - EndpointURLS.REGISTRY_DISCOVERY_ENDPOINT: "https://{}{}.api.azureml.{}/".format( - cloud_name.lower(), - registry_discovery_region, - cloud_suffix - ) + EndpointURLS.REGISTRY_DISCOVERY_ENDPOINT: registry_discovery_url } except KeyError as ex: module_logger.warning("Property on cloud not found in arm cloud metadata: %s", ex) diff --git a/sdk/ml/azure-ai-ml/azure/ai/ml/constants/_common.py b/sdk/ml/azure-ai-ml/azure/ai/ml/constants/_common.py index 6d7b23ce5a0b..4f2ea1764bad 100644 --- a/sdk/ml/azure-ai-ml/azure/ai/ml/constants/_common.py +++ b/sdk/ml/azure-ai-ml/azure/ai/ml/constants/_common.py @@ -278,6 +278,7 @@ class ArmConstants(object): METADATA_URL_ENV_NAME = "ARM_CLOUD_METADATA_URL" REGISTRY_DISCOVERY_DEFAULT_REGION = "west" REGISTRY_DISCOVERY_REGION_ENV_NAME = "REGISTRY_DISCOVERY_ENDPOINT_REGION" + REGISTRY_ENV_URL = "REGISTRY_DISCOVERY_ENDPOINT_URL" class HttpResponseStatusCode(object): diff --git a/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py b/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py index 7e3c123b7773..d792cdc052e5 100644 --- a/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py +++ b/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py @@ -168,7 +168,7 @@ def test_all_endpointurls_used(self, mock_get): def test_metadata_registry_endpoint(self, mock_get): cloud_details = _get_cloud_details("TEST_ENV2") assert cloud_details.get(EndpointURLS.REGISTRY_DISCOVERY_ENDPOINT) == "https://test_env2west.api.azureml.windows.net/" - + @mock.patch.dict(os.environ, {}, clear=True) @mock.patch('requests.get', side_effect=mock_arm_response) def test_arm_misconfigured(self, mock_get): From 848a871c7d38430a5d0db1c36c6438244d4f08e1 Mon Sep 17 00:00:00 2001 From: Matt Brown Date: Mon, 6 Feb 2023 09:37:31 -0500 Subject: [PATCH 20/24] Fixing small pylint errors --- .../azure/ai/ml/_azure_environments.py | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py index 7ebcd537bf7a..f56a59e1068a 100644 --- a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py +++ b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py @@ -206,18 +206,17 @@ def _get_registry_discovery_url(cloud, cloud_suffix): cloud_name = cloud["name"] if cloud_name in _environments: return _environments[cloud_name].registry_url - elif ArmConstants.REGISTRY_ENV_URL in os.environ: + if ArmConstants.REGISTRY_ENV_URL in os.environ: return os.environ[ArmConstants.REGISTRY_ENV_URL] - else: - registry_discovery_region = os.environ.get( - ArmConstants.REGISTRY_DISCOVERY_REGION_ENV_NAME, - ArmConstants.REGISTRY_DISCOVERY_DEFAULT_REGION - ) - return "https://{}{}.api.azureml.{}/".format( - cloud_name.lower(), - registry_discovery_region, - cloud_suffix - ) + registry_discovery_region = os.environ.get( + ArmConstants.REGISTRY_DISCOVERY_REGION_ENV_NAME, + ArmConstants.REGISTRY_DISCOVERY_DEFAULT_REGION + ) + return "https://{}{}.api.azureml.{}/".format( + cloud_name.lower(), + registry_discovery_region, + cloud_suffix + ) def _get_clouds_by_metadata_url(metadata_url, timeout=ArmConstants.DEFAULT_TIMEOUT): """Get all the clouds by the specified metadata url From 1ca9fec4a1a7ff36b2160278d9c2d0d0d0d18606 Mon Sep 17 00:00:00 2001 From: Matt Brown Date: Mon, 6 Feb 2023 16:50:02 -0500 Subject: [PATCH 21/24] Moving over to using HttpPipeline instead of requests --- .../azure/ai/ml/_azure_environments.py | 18 +++- .../unittests/test_cloud_environments.py | 97 ++++++++----------- 2 files changed, 53 insertions(+), 62 deletions(-) diff --git a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py index f56a59e1068a..9138454c0edd 100644 --- a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py +++ b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py @@ -9,8 +9,11 @@ from typing import Dict, Optional from azure.ai.ml._utils.utils import _get_mfe_url_override +from azure.ai.ml._utils._http_utils import HttpPipeline from azure.ai.ml.constants._common import AZUREML_CLOUD_ENV_NAME from azure.ai.ml.constants._common import ArmConstants +from azure.core.rest import HttpRequest +from azure.mgmt.core import ARMPipelineClient module_logger = logging.getLogger(__name__) @@ -57,6 +60,8 @@ class EndpointURLS: # pylint: disable=too-few-public-methods,no-init }, } +_requests_pipeline = None + def _get_cloud(cloud: str): if cloud in _environments: return _environments[cloud] @@ -199,8 +204,11 @@ def _resource_to_scopes(resource): scope = resource + "/.default" return [scope] -def _get_registry_discovery_url(cloud, cloud_suffix): - """Get the registry discovery url +def _get_registry_discovery_url(cloud, cloud_suffix=""): + """Get or enerate the registry discovery url + + :param cloud: configuration of the cloud to get the registry_discovery_url from + :param cloud_suffix: the suffix to use for the cloud, in the case that the registry_discovery_url must be generated :return: string of discovery url """ cloud_name = cloud["name"] @@ -223,12 +231,12 @@ def _get_clouds_by_metadata_url(metadata_url, timeout=ArmConstants.DEFAULT_TIMEO :return: list of the clouds """ - import requests - try: module_logger.debug('Start : Loading cloud metadata from the url specified by %s', metadata_url) - with requests.get(metadata_url, timeout=timeout) as meta_response: + client = ARMPipelineClient(base_url=metadata_url, policies=[]) + with client.send_request(HttpRequest("GET", metadata_url)) as meta_response: arm_cloud_dict = meta_response.json() + print("arm_cloud_dict", arm_cloud_dict) cli_cloud_dict = _convert_arm_to_cli(arm_cloud_dict) module_logger.debug('Finish : Loading cloud metadata from the url specified by %s', metadata_url) return cli_cloud_dict diff --git a/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py b/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py index d792cdc052e5..5fa6cfffb128 100644 --- a/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py +++ b/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py @@ -17,11 +17,43 @@ _set_cloud, ) from azure.ai.ml.constants._common import ArmConstants, AZUREML_CLOUD_ENV_NAME - - - - - +from azure.mgmt.core import ARMPipelineClient + +class MockResponse: + def __init__(self): + self.status_code = 201 + def __enter__(self): + return self + def __exit__(self, exc_type, exc_value, traceback): + return + def json(self): + return [ + { + "name": "TEST_ENV", + "portal": "testportal.azure.com", + "resourceManager": "testresourcemanager.azure.com", + "authentication": { + "loginEndpoint": "testdirectoryendpoint.azure.com" + }, + "suffixes": { + "storage": "teststorageendpoint" + } + }, + { + "name": "TEST_ENV2", + "portal": "testportal.azure.windows.net", + "resourceManager": "testresourcemanager.azure.com", + "authentication": { + "loginEndpoint": "testdirectoryendpoint.azure.com" + }, + "suffixes": { + "storage": "teststorageendpoint" + } + }, + { + "name": "MISCONFIGURED" + } + ] @pytest.mark.unittest @pytest.mark.core_sdk_test class TestCloudEnvironments: @@ -97,62 +129,15 @@ def test_get_registry_endpoint_from_us_gov(self): base_url = _get_registry_discovery_endpoint_from_metadata(cloud_environment) assert "https://usgovarizona.api.ml.azure.us/" in base_url - - - - def mock_arm_response(*args, **kwargs): - class MockResponse: - def __init__(self, json_data): - self.json_data = json_data - self.status_code = 201 - def __enter__(self): - return self; - - def __exit__(self, exc_type, exc_value, traceback): - return - - def json(self): - return self.json_data - - json_data = [ - { - "name": "TEST_ENV", - "portal": "testportal.azure.com", - "resourceManager": "testresourcemanager.azure.com", - "authentication": { - "loginEndpoint": "testdirectoryendpoint.azure.com" - }, - "suffixes": { - "storage": "teststorageendpoint" - } - }, - { - "name": "TEST_ENV2", - "portal": "testportal.azure.windows.net", - "resourceManager": "testresourcemanager.azure.com", - "authentication": { - "loginEndpoint": "testdirectoryendpoint.azure.com" - }, - "suffixes": { - "storage": "teststorageendpoint" - } - }, - { - "name": "MISCONFIGURED" - } - ] - response = MockResponse(json_data) - return response - @mock.patch.dict(os.environ, {}, clear=True) - @mock.patch('requests.get', side_effect=mock_arm_response) - def test_get_cloud_from_arm(self, mock_get): + @mock.patch("azure.mgmt.core.ARMPipelineClient", "send_request", return_value=MockResponse()) + def test_get_cloud_from_arm(self, mock_arm_pipeline_client): + print("mock_client base_url", mock_arm_pipeline_client._base_url) _set_cloud('TEST_ENV') cloud_details = _get_cloud_information_from_metadata("TEST_ENV") assert cloud_details.get("cloud") == "TEST_ENV" @mock.patch.dict(os.environ, {}, clear=True) - @mock.patch('requests.get', side_effect=mock_arm_response) def test_all_endpointurls_used(self, mock_get): cloud_details = _get_cloud_details("TEST_ENV") endpoint_urls = [a for a in dir(EndpointURLS) if not a.startswith('__')] @@ -164,13 +149,11 @@ def test_all_endpointurls_used(self, mock_get): assert True @mock.patch.dict(os.environ, {}, clear=True) - @mock.patch('requests.get', side_effect=mock_arm_response) def test_metadata_registry_endpoint(self, mock_get): cloud_details = _get_cloud_details("TEST_ENV2") assert cloud_details.get(EndpointURLS.REGISTRY_DISCOVERY_ENDPOINT) == "https://test_env2west.api.azureml.windows.net/" @mock.patch.dict(os.environ, {}, clear=True) - @mock.patch('requests.get', side_effect=mock_arm_response) def test_arm_misconfigured(self, mock_get): with pytest.raises(Exception) as e_info: _set_cloud("MISCONFIGURED") From 55964d323bf5a364361168c9df4fc1906b43285c Mon Sep 17 00:00:00 2001 From: Ronald Shaw Date: Mon, 6 Feb 2023 16:50:48 -0500 Subject: [PATCH 22/24] fixed up based on comments in the PR --- .../azure/ai/ml/_azure_environments.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py index f56a59e1068a..09f6ee57b260 100644 --- a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py +++ b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py @@ -12,6 +12,7 @@ from azure.ai.ml.constants._common import AZUREML_CLOUD_ENV_NAME from azure.ai.ml.constants._common import ArmConstants + module_logger = logging.getLogger(__name__) @@ -90,6 +91,10 @@ def _get_cloud_details(cloud: str = AzureEnvironments.ENV_DEFAULT): def _set_cloud(cloud: str = AzureEnvironments.ENV_DEFAULT): + """Sets the current cloud + + :param cloud: cloud name + """ if cloud is not None: try: _get_cloud(cloud) @@ -201,22 +206,27 @@ def _resource_to_scopes(resource): def _get_registry_discovery_url(cloud, cloud_suffix): """Get the registry discovery url + + :param cloud: The cloud to retrieve from + :param cloud_suffix: the url suffix of the cloud :return: string of discovery url """ cloud_name = cloud["name"] if cloud_name in _environments: return _environments[cloud_name].registry_url - if ArmConstants.REGISTRY_ENV_URL in os.environ: - return os.environ[ArmConstants.REGISTRY_ENV_URL] + registry_discovery_region = os.environ.get( ArmConstants.REGISTRY_DISCOVERY_REGION_ENV_NAME, ArmConstants.REGISTRY_DISCOVERY_DEFAULT_REGION ) - return "https://{}{}.api.azureml.{}/".format( + registry_discovery_region_default = "https://{}{}.api.azureml.{}/".format( cloud_name.lower(), registry_discovery_region, cloud_suffix ) + return os.environ.get(ArmConstants.REGISTRY_ENV_URL, registry_discovery_region_default) + + def _get_clouds_by_metadata_url(metadata_url, timeout=ArmConstants.DEFAULT_TIMEOUT): """Get all the clouds by the specified metadata url From ee52f1331ce253b8f56be1bbc2889edfdd574af9 Mon Sep 17 00:00:00 2001 From: Ronald Shaw Date: Mon, 6 Feb 2023 19:16:54 -0500 Subject: [PATCH 23/24] fixed broken unit tests and mocked correctly --- .../azure/ai/ml/_azure_environments.py | 5 +- .../unittests/test_cloud_environments.py | 77 ++++++++++--------- 2 files changed, 46 insertions(+), 36 deletions(-) diff --git a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py index a2d3598bce71..16c7fd3ec56e 100644 --- a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py +++ b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py @@ -15,6 +15,9 @@ from azure.core.rest import HttpRequest from azure.mgmt.core import ARMPipelineClient +from pprint import pformat + + module_logger = logging.getLogger(__name__) @@ -241,9 +244,9 @@ def _get_clouds_by_metadata_url(metadata_url, timeout=ArmConstants.DEFAULT_TIMEO try: module_logger.debug('Start : Loading cloud metadata from the url specified by %s', metadata_url) client = ARMPipelineClient(base_url=metadata_url, policies=[]) + HttpRequest("GET", metadata_url) with client.send_request(HttpRequest("GET", metadata_url)) as meta_response: arm_cloud_dict = meta_response.json() - print("arm_cloud_dict", arm_cloud_dict) cli_cloud_dict = _convert_arm_to_cli(arm_cloud_dict) module_logger.debug('Finish : Loading cloud metadata from the url specified by %s', metadata_url) return cli_cloud_dict diff --git a/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py b/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py index 5fa6cfffb128..d82629c78e3e 100644 --- a/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py +++ b/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py @@ -19,41 +19,45 @@ from azure.ai.ml.constants._common import ArmConstants, AZUREML_CLOUD_ENV_NAME from azure.mgmt.core import ARMPipelineClient -class MockResponse: - def __init__(self): - self.status_code = 201 - def __enter__(self): - return self - def __exit__(self, exc_type, exc_value, traceback): - return - def json(self): - return [ - { - "name": "TEST_ENV", - "portal": "testportal.azure.com", - "resourceManager": "testresourcemanager.azure.com", - "authentication": { - "loginEndpoint": "testdirectoryendpoint.azure.com" +def mocked_send_request_get(*args, **kwargs): + class MockResponse: + def __init__(self): + self.status_code = 201 + def __enter__(self): + return self + def __exit__(self, exc_type, exc_value, traceback): + return + def json(self): + return [ + { + "name": "TEST_ENV", + "portal": "testportal.azure.com", + "resourceManager": "testresourcemanager.azure.com", + "authentication": { + "loginEndpoint": "testdirectoryendpoint.azure.com" + }, + "suffixes": { + "storage": "teststorageendpoint" + } }, - "suffixes": { - "storage": "teststorageendpoint" - } - }, - { - "name": "TEST_ENV2", - "portal": "testportal.azure.windows.net", - "resourceManager": "testresourcemanager.azure.com", - "authentication": { - "loginEndpoint": "testdirectoryendpoint.azure.com" + { + "name": "TEST_ENV2", + "portal": "testportal.azure.windows.net", + "resourceManager": "testresourcemanager.azure.com", + "authentication": { + "loginEndpoint": "testdirectoryendpoint.azure.com" + }, + "suffixes": { + "storage": "teststorageendpoint" + } }, - "suffixes": { - "storage": "teststorageendpoint" + { + "name": "MISCONFIGURED" } - }, - { - "name": "MISCONFIGURED" - } - ] + ] + return MockResponse() + + @pytest.mark.unittest @pytest.mark.core_sdk_test class TestCloudEnvironments: @@ -130,14 +134,15 @@ def test_get_registry_endpoint_from_us_gov(self): assert "https://usgovarizona.api.ml.azure.us/" in base_url @mock.patch.dict(os.environ, {}, clear=True) - @mock.patch("azure.mgmt.core.ARMPipelineClient", "send_request", return_value=MockResponse()) - def test_get_cloud_from_arm(self, mock_arm_pipeline_client): - print("mock_client base_url", mock_arm_pipeline_client._base_url) + @mock.patch("azure.mgmt.core.ARMPipelineClient.send_request", side_effect=mocked_send_request_get) + def test_get_cloud_from_arm(self, mock_arm_pipeline_client_send_request): + _set_cloud('TEST_ENV') cloud_details = _get_cloud_information_from_metadata("TEST_ENV") assert cloud_details.get("cloud") == "TEST_ENV" @mock.patch.dict(os.environ, {}, clear=True) + @mock.patch("azure.mgmt.core.ARMPipelineClient.send_request", side_effect=mocked_send_request_get) def test_all_endpointurls_used(self, mock_get): cloud_details = _get_cloud_details("TEST_ENV") endpoint_urls = [a for a in dir(EndpointURLS) if not a.startswith('__')] @@ -149,11 +154,13 @@ def test_all_endpointurls_used(self, mock_get): assert True @mock.patch.dict(os.environ, {}, clear=True) + @mock.patch("azure.mgmt.core.ARMPipelineClient.send_request", side_effect=mocked_send_request_get) def test_metadata_registry_endpoint(self, mock_get): cloud_details = _get_cloud_details("TEST_ENV2") assert cloud_details.get(EndpointURLS.REGISTRY_DISCOVERY_ENDPOINT) == "https://test_env2west.api.azureml.windows.net/" @mock.patch.dict(os.environ, {}, clear=True) + @mock.patch("azure.mgmt.core.ARMPipelineClient.send_request", side_effect=mocked_send_request_get) def test_arm_misconfigured(self, mock_get): with pytest.raises(Exception) as e_info: _set_cloud("MISCONFIGURED") From f94358ed9dc125aa49a1cc73c61572a58b1fe95a Mon Sep 17 00:00:00 2001 From: Matt Brown Date: Tue, 7 Feb 2023 08:27:44 -0500 Subject: [PATCH 24/24] Fixing pylint issues --- .../azure-ai-ml/azure/ai/ml/_azure_environments.py | 14 +++++--------- .../azure-ai-ml/azure/ai/ml/constants/_common.py | 1 - .../unittests/test_cloud_environments.py | 2 -- 3 files changed, 5 insertions(+), 12 deletions(-) diff --git a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py index 16c7fd3ec56e..c42317356990 100644 --- a/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py +++ b/sdk/ml/azure-ai-ml/azure/ai/ml/_azure_environments.py @@ -9,14 +9,11 @@ from typing import Dict, Optional from azure.ai.ml._utils.utils import _get_mfe_url_override -from azure.ai.ml._utils._http_utils import HttpPipeline from azure.ai.ml.constants._common import AZUREML_CLOUD_ENV_NAME from azure.ai.ml.constants._common import ArmConstants from azure.core.rest import HttpRequest from azure.mgmt.core import ARMPipelineClient -from pprint import pformat - module_logger = logging.getLogger(__name__) @@ -100,7 +97,7 @@ def _get_cloud_details(cloud: str = AzureEnvironments.ENV_DEFAULT): def _set_cloud(cloud: str = AzureEnvironments.ENV_DEFAULT): """Sets the current cloud - + :param cloud: cloud name """ if cloud is not None: @@ -216,13 +213,14 @@ def _get_registry_discovery_url(cloud, cloud_suffix=""): """Get or generate the registry discovery url :param cloud: configuration of the cloud to get the registry_discovery_url from - :param cloud_suffix: the suffix to use for the cloud, in the case that the registry_discovery_url must be generated + :param cloud_suffix: the suffix to use for the cloud, in the case that the registry_discovery_url + must be generated :return: string of discovery url """ cloud_name = cloud["name"] if cloud_name in _environments: return _environments[cloud_name].registry_url - + registry_discovery_region = os.environ.get( ArmConstants.REGISTRY_DISCOVERY_REGION_ENV_NAME, ArmConstants.REGISTRY_DISCOVERY_DEFAULT_REGION @@ -233,10 +231,8 @@ def _get_registry_discovery_url(cloud, cloud_suffix=""): cloud_suffix ) return os.environ.get(ArmConstants.REGISTRY_ENV_URL, registry_discovery_region_default) - - -def _get_clouds_by_metadata_url(metadata_url, timeout=ArmConstants.DEFAULT_TIMEOUT): +def _get_clouds_by_metadata_url(metadata_url): """Get all the clouds by the specified metadata url :return: list of the clouds diff --git a/sdk/ml/azure-ai-ml/azure/ai/ml/constants/_common.py b/sdk/ml/azure-ai-ml/azure/ai/ml/constants/_common.py index 4f2ea1764bad..fd01fd4ca5e9 100644 --- a/sdk/ml/azure-ai-ml/azure/ai/ml/constants/_common.py +++ b/sdk/ml/azure-ai-ml/azure/ai/ml/constants/_common.py @@ -273,7 +273,6 @@ class ArmConstants(object): AZURE_MGMT_KEYVAULT_API_VERSION = "2019-09-01" AZURE_MGMT_CONTAINER_REG_API_VERSION = "2019-05-01" - DEFAULT_TIMEOUT = 30 DEFAULT_URL = "https://management.azure.com/metadata/endpoints?api-version=2019-05-01" METADATA_URL_ENV_NAME = "ARM_CLOUD_METADATA_URL" REGISTRY_DISCOVERY_DEFAULT_REGION = "west" diff --git a/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py b/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py index d82629c78e3e..93f0df644dee 100644 --- a/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py +++ b/sdk/ml/azure-ai-ml/tests/internal_utils/unittests/test_cloud_environments.py @@ -2,7 +2,6 @@ import mock import pytest from mock import MagicMock, patch -import requests from azure.ai.ml._azure_environments import ( AzureEnvironments, @@ -113,7 +112,6 @@ def test_get_default_cloud(self): with mock.patch("os.environ", {AZUREML_CLOUD_ENV_NAME: "yadadada"}): cloud_name = _get_default_cloud_name() assert cloud_name == "yadadada" - def test_get_registry_endpoint_from_public(self): cloud_environment = AzureEnvironments.ENV_DEFAULT