Skip to content

Commit

Permalink
[promptflow][bugfix] Fix SubscriptionNotFound in Azure replay test (#…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
zhengfeiwang authored Jan 10, 2024
1 parent 71cc2cf commit ee636fc
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 7 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/sdk-cli-azure-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion scripts/building/check_enforcer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand Down
6 changes: 2 additions & 4 deletions src/promptflow/promptflow/azure/operations/_run_operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
27 changes: 26 additions & 1 deletion src/promptflow/tests/sdk_cli_azure_test/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

0 comments on commit ee636fc

Please sign in to comment.