From ee636fc644e6a5a41f9aa473d7befe1f099beea2 Mon Sep 17 00:00:00 2001 From: Zhengfei Wang <38847871+zhengfeiwang@users.noreply.github.com> Date: Wed, 10 Jan 2024 19:19:30 +0800 Subject: [PATCH] [promptflow][bugfix] Fix `SubscriptionNotFound` in Azure replay test (#1697) # Description For unknown reason, lots of Azure tests run into `SubscriptionNotFound` between January 9th~10th (an example [workflow run](https://github.com/microsoft/promptflow/actions/runs/7472614865/job/20335205209)). With some investigation, it seems there is something wrong during upload local data and resolve as remote uri. Given this context, this PR targets to mock this process **ONLY** in replay test, so that it won't break any more. This PR also revert the change that makes the Azure tests optional. # All Promptflow Contribution checklist: - [x] **The pull request does not introduce [breaking changes].** - [ ] **CHANGELOG is updated for new features, bug fixes or other significant changes.** - [x] **I have read the [contribution guidelines](../CONTRIBUTING.md).** - [ ] **Create an issue and link to the pull request to get dedicated review from promptflow team. Learn more: [suggested workflow](../CONTRIBUTING.md#suggested-workflow).** ## General Guidelines and Best Practices - [x] Title of the pull request is clear and informative. - [x] There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, [see this page](https://github.com/Azure/azure-powershell/blob/master/documentation/development-docs/cleaning-up-commits.md). ### Testing Guidelines - [ ] Pull request includes test coverage for the included changes. --- .github/workflows/sdk-cli-azure-test.yml | 2 +- scripts/building/check_enforcer.py | 2 +- .../azure/operations/_run_operations.py | 6 ++--- .../tests/sdk_cli_azure_test/conftest.py | 27 ++++++++++++++++++- 4 files changed, 30 insertions(+), 7 deletions(-) diff --git a/.github/workflows/sdk-cli-azure-test.yml b/.github/workflows/sdk-cli-azure-test.yml index 4cbf98b6145..a2893a94112 100644 --- a/.github/workflows/sdk-cli-azure-test.yml +++ b/.github/workflows/sdk-cli-azure-test.yml @@ -9,7 +9,7 @@ on: paths: - src/promptflow/** - scripts/building/** - - .github/workflows/promptflow-sdk-cli-azure-test.yml + - .github/workflows/sdk-cli-azure-test.yml schedule: - cron: "30 20 * * *" # 4:30 Beijing Time (GMT+8) diff --git a/scripts/building/check_enforcer.py b/scripts/building/check_enforcer.py index 8c80a95f668..2722a570698 100644 --- a/scripts/building/check_enforcer.py +++ b/scripts/building/check_enforcer.py @@ -36,7 +36,7 @@ # Please notice that the key should be the Job Name in the pipeline. special_care = { "sdk_cli_tests": 4, - "sdk_cli_azure_test": 0, + "sdk_cli_azure_test": 4, # "samples_connections_connection": 0, } diff --git a/src/promptflow/promptflow/azure/operations/_run_operations.py b/src/promptflow/promptflow/azure/operations/_run_operations.py index 69ab643bb7c..b36723b7620 100644 --- a/src/promptflow/promptflow/azure/operations/_run_operations.py +++ b/src/promptflow/promptflow/azure/operations/_run_operations.py @@ -17,13 +17,14 @@ import requests import yaml +from azure.ai.ml._artifacts._artifact_utilities import _upload_and_generate_remote_uri from azure.ai.ml._scope_dependent_operations import ( OperationConfig, OperationsContainer, OperationScope, _ScopeDependentOperations, ) -from azure.ai.ml.constants._common import AzureMLResourceType +from azure.ai.ml.constants._common import AssetTypes, AzureMLResourceType from azure.ai.ml.entities import Workspace from azure.ai.ml.operations import DataOperations from azure.ai.ml.operations._operation_orchestrator import OperationOrchestrator @@ -673,9 +674,6 @@ def stream(self, run: Union[str, Run], raise_on_error: bool = True) -> Run: return run def _resolve_data_to_asset_id(self, run: Run): - from azure.ai.ml._artifacts._artifact_utilities import _upload_and_generate_remote_uri - from azure.ai.ml.constants._common import AssetTypes - # Skip if no data provided if run.data is None: return diff --git a/src/promptflow/tests/sdk_cli_azure_test/conftest.py b/src/promptflow/tests/sdk_cli_azure_test/conftest.py index 623f34d5eda..eb8e1610600 100644 --- a/src/promptflow/tests/sdk_cli_azure_test/conftest.py +++ b/src/promptflow/tests/sdk_cli_azure_test/conftest.py @@ -7,7 +7,7 @@ import uuid from concurrent.futures import ThreadPoolExecutor from pathlib import Path -from typing import Callable, Optional +from typing import Callable, Optional, Union from unittest.mock import patch import jwt @@ -488,3 +488,28 @@ def mock_isinstance(*args): with patch("builtins.isinstance", new=mock_isinstance): yield + + +@pytest.fixture(autouse=is_replay()) +def mock_upload_and_generate_remote_uri() -> None: + from azure.ai.ml._scope_dependent_operations import OperationScope + from azure.ai.ml.entities._datastore._constants import WORKSPACE_BLOB_STORE + from azure.ai.ml.exceptions import ErrorTarget + from azure.ai.ml.operations._datastore_operations import DatastoreOperations + + def _upload_and_generate_remote_uri( + operation_scope: OperationScope, + datastore_operation: DatastoreOperations, + path: Union[str, Path, os.PathLike], + artifact_type: str = ErrorTarget.ARTIFACT, + datastore_name: str = WORKSPACE_BLOB_STORE, + show_progress: bool = True, + ) -> str: + path = Path(path).resolve() + return f"azureml://datastores/{datastore_name}/paths/LocalUpload/00000000000000000000000000000000/{path.name}" + + with patch( + "promptflow.azure.operations._run_operations._upload_and_generate_remote_uri", + new=_upload_and_generate_remote_uri, + ): + yield