[evaluation] ci: Re-enable Windows tests (Azure#37411)

* ci: Revert back to repo's platform matrix

* fix: Wrap async clients in async with blocks

* tests,fix: Ensure that promptflow service is stopped

Author: kdestin

sdk/evaluation/azure-ai-evaluation/azure/ai/evaluation/_common/rai_service.py

@@ -65,11 +65,10 @@ async def ensure_service_availability(rai_svc_url: str, token: str, capability:
     headers = get_common_headers(token)
     svc_liveness_url = rai_svc_url + "/checkannotation"
 
-    client = get_async_http_client()
-
-    response = await client.get(  # pylint: disable=too-many-function-args,unexpected-keyword-arg
-        svc_liveness_url, headers=headers, timeout=CommonConstants.DEFAULT_HTTP_TIMEOUT
-    )
+    async with get_async_http_client() as client:
+        response = await client.get(  # pylint: disable=too-many-function-args,unexpected-keyword-arg
+            svc_liveness_url, headers=headers, timeout=CommonConstants.DEFAULT_HTTP_TIMEOUT
+        )
 
     if response.status_code != 200:
         raise Exception(  # pylint: disable=broad-exception-raised
@@ -143,11 +142,10 @@ async def submit_request(question: str, answer: str, metric: str, rai_svc_url: s
     url = rai_svc_url + "/submitannotation"
     headers = get_common_headers(token)
 
-    client = get_async_http_client()
-
-    response = await client.post(  # pylint: disable=too-many-function-args,unexpected-keyword-arg
-        url, json=payload, headers=headers, timeout=CommonConstants.DEFAULT_HTTP_TIMEOUT
-    )
+    async with get_async_http_client() as client:
+        response = await client.post(  # pylint: disable=too-many-function-args,unexpected-keyword-arg
+            url, json=payload, headers=headers, timeout=CommonConstants.DEFAULT_HTTP_TIMEOUT
+        )
 
     if response.status_code != 202:
         print("Fail evaluating '%s' with error message: %s" % (payload["UserTextList"], response.text))
@@ -180,11 +178,10 @@ async def fetch_result(operation_id: str, rai_svc_url: str, credential: TokenCre
         token = await fetch_or_reuse_token(credential, token)
         headers = get_common_headers(token)
 
-        client = get_async_http_client()
-
-        response = await client.get(  # pylint: disable=too-many-function-args,unexpected-keyword-arg
-            url, headers=headers, timeout=CommonConstants.DEFAULT_HTTP_TIMEOUT
-        )
+        async with get_async_http_client() as client:
+            response = await client.get(  # pylint: disable=too-many-function-args,unexpected-keyword-arg
+                url, headers=headers, timeout=CommonConstants.DEFAULT_HTTP_TIMEOUT
+            )
 
         if response.status_code == 200:
             return response.json()
@@ -342,16 +339,15 @@ async def _get_service_discovery_url(azure_ai_project: dict, token: str) -> str:
     """
     headers = get_common_headers(token)
 
-    client = get_async_http_client()
-
-    response = await client.get(  # pylint: disable=too-many-function-args,unexpected-keyword-arg
-        f"https://management.azure.com/subscriptions/{azure_ai_project['subscription_id']}/"
-        f"resourceGroups/{azure_ai_project['resource_group_name']}/"
-        f"providers/Microsoft.MachineLearningServices/workspaces/{azure_ai_project['project_name']}?"
-        f"api-version=2023-08-01-preview",
-        headers=headers,
-        timeout=CommonConstants.DEFAULT_HTTP_TIMEOUT,
-    )
+    async with get_async_http_client() as client:
+        response = await client.get(  # pylint: disable=too-many-function-args,unexpected-keyword-arg
+            f"https://management.azure.com/subscriptions/{azure_ai_project['subscription_id']}/"
+            f"resourceGroups/{azure_ai_project['resource_group_name']}/"
+            f"providers/Microsoft.MachineLearningServices/workspaces/{azure_ai_project['project_name']}?"
+            f"api-version=2023-08-01-preview",
+            headers=headers,
+            timeout=CommonConstants.DEFAULT_HTTP_TIMEOUT,
+        )
 
     if response.status_code != 200:
         raise Exception("Failed to retrieve the discovery service URL")  # pylint: disable=broad-exception-raised

sdk/evaluation/azure-ai-evaluation/azure/ai/evaluation/simulator/_adversarial_simulator.py

@@ -321,7 +321,7 @@ async def _simulate_async(
             )
         )
 
-        async with semaphore:
+        async with semaphore, session:
             _, conversation_history = await simulate_conversation(
                 bots=bots,
                 session=session,

sdk/evaluation/azure-ai-evaluation/azure/ai/evaluation/simulator/_model_tools/_proxy_completion_model.py

@@ -196,17 +196,16 @@ async def request_api(
             retry_mode=RetryMode.Exponential,
         )
 
-        exp_retry_client = get_async_http_client().with_policies(retry_policy=retry_policy)
-
         # initial 15 seconds wait before attempting to fetch result
         # Need to wait both in this thread and in the async thread for some reason?
         # Someone not under a crunch and with better async understandings should dig into this more.
         await asyncio.sleep(15)
         time.sleep(15)
 
-        response = await exp_retry_client.get(  # pylint: disable=too-many-function-args,unexpected-keyword-arg
-            self.result_url, headers=proxy_headers
-        )
+        async with get_async_http_client().with_policies(retry_policy=retry_policy) as exp_retry_client:
+            response = await exp_retry_client.get(  # pylint: disable=too-many-function-args,unexpected-keyword-arg
+                self.result_url, headers=proxy_headers
+            )
 
         response.raise_for_status()
 

sdk/evaluation/azure-ai-evaluation/azure/ai/evaluation/simulator/_model_tools/_rai_client.py

@@ -122,7 +122,9 @@ async def get(self, url: str) -> Any:
         }
 
         session = self._create_async_client()
-        response = await session.get(url=url, headers=headers)  # pylint: disable=unexpected-keyword-arg
+
+        async with session:
+            response = await session.get(url=url, headers=headers)  # pylint: disable=unexpected-keyword-arg
 
         if response.status_code == 200:
             return response.json()

sdk/evaluation/azure-ai-evaluation/tests/conftest.py

@@ -471,3 +471,28 @@ def pytest_collection_modifyitems(items):
                 # If item's parent was marked as 'localtest', mark the child as such, but not if
                 # it was marked as 'azuretest'.
                 item.add_marker(pytest.mark.localtest)
+
+
+def pytest_sessionfinish() -> None:
+
+    def stop_promptflow_service() -> None:
+        """Ensure that the promptflow service is stopped when pytest exits.
+
+        .. note::
+
+            The azure-sdk-for-python CI performs a cleanup step that deletes
+            the python environment that the tests run in.
+
+            At time of writing, at least one test starts the promptflow service
+            (served from `waitress-serve`). The promptflow service is a separate
+            process that gets orphaned by pytest.
+
+            Crucially, that process has a handles on files in the python environment.
+            On Windows, this causes the cleanup step to fail with a permission issue
+            since the OS disallows deletion of files in use by a process.
+        """
+        from promptflow._cli._pf._service import stop_service
+
+        stop_service()
+
+    stop_promptflow_service()

sdk/evaluation/azure-ai-evaluation/tests/unittests/test_synthetic_conversation_bot.py

@@ -84,9 +84,11 @@ async def test_conversation_bot_initialization_user_invalid_jinja(self, bot_inva
             )
         )
 
-        parsed_response, req, time_taken, full_response = await bot.generate_response(
-            session=client, conversation_history=[], max_history=0, turn_number=0
-        )
+        async with client:
+            parsed_response, req, time_taken, full_response = await bot.generate_response(
+                session=client, conversation_history=[], max_history=0, turn_number=0
+            )
+
         assert (
             parsed_response["samples"][0]
             == bot_invalid_jinja_params["instantiation_parameters"]["conversation_starter"]

sdk/evaluation/ci.yml

@@ -28,13 +28,6 @@ extends:
     ServiceDirectory: evaluation
     ValidateFormatting: true
     TestProxy: true
-    # This custom matrix config should be dropped once:
-    #  * Resolve the issue of windows runners crashing because a file isn't deletable
-    MatrixConfigs: 
-      - Name: ai_ci_matrix
-        Path: sdk/evaluation/platform-matrix.json
-        Selection: sparse
-        GenerateVMJobs: true
     Artifacts:
     - name: azure-ai-evaluation
       safeName: azureaievaluation