Fix workspace arm template (#35931)

* new tests and fix template

* add skips

* remove whitespace

* fix full skip

* run black

Author: MilesHolland

sdk/ml/azure-ai-ml/assets.json

@@ -2,5 +2,5 @@
   "AssetsRepo": "Azure/azure-sdk-assets",
   "AssetsRepoPrefixPath": "python",
   "TagPrefix": "python/ml/azure-ai-ml",
-  "Tag": "python/ml/azure-ai-ml_ae30eb5b40"
+  "Tag": "python/ml/azure-ai-ml_67cbe1a754"
 }

sdk/ml/azure-ai-ml/azure/ai/ml/operations/_workspace_operations_base.py

@@ -148,8 +148,6 @@ def begin_create(
             workspace.tags = {}
         if workspace.tags.get("createdByToolkit") is None:
             workspace.tags["createdByToolkit"] = "sdk-v2-{}".format(VERSION)
-        if workspace.tags.get("AttachAppInsightsToWorkspace") is None:
-            workspace.tags["AttachAppInsightsToWorkspace"] = "false"
 
         workspace.resource_group = resource_group
         (

sdk/ml/azure-ai-ml/tests/workspace/e2etests/test_workspace.py

@@ -31,13 +31,13 @@
     "recorded_test", "mock_workspace_arm_template_deployment_name", "mock_workspace_dependent_resource_name_generator"
 )
 class TestWorkspace(AzureRecordedTestCase):
+
+    # WARNING: This test takes a long time to run in live mode.
     @pytest.mark.skipif(
         condition=not is_live(),
         reason="ARM template makes playback complex, so the test is flaky when run against recording",
     )
-    def test_workspace_create_update_and_delete(
-        self, client: MLClient, randstr: Callable[[], str], location: str
-    ) -> None:
+    def test_workspace_create_and_delete(self, client: MLClient, randstr: Callable[[], str], location: str) -> None:
         wps_name = f"e2etest_{randstr('wps_name')}"
         wps_description = f"{wps_name} description"
         wps_display_name = f"{wps_name} display name"
@@ -77,8 +77,55 @@ def workspace_validation(wps):
         assert isinstance(workspace, Workspace)
         assert workspace.name == wps_name
 
-        static_acr = "/subscriptions/8f338f6e-4fce-44ae-969c-fc7d8fda030e/resourceGroups/rg-mhe-e2e-test-dont-remove/providers/Microsoft.ContainerRegistry/registries/acrmhetest2"
-        static_appinsights = "/subscriptions/8f338f6e-4fce-44ae-969c-fc7d8fda030e/resourceGroups/rg-mhe-e2e-test-dont-remove/providers/microsoft.insights/components/aimhetest2"
+        poller = client.workspaces.begin_delete(wps_name, delete_dependent_resources=True, permanently_delete=True)
+        # verify that request was accepted by checking if poller is returned
+        assert poller
+        assert isinstance(poller, LROPoller)
+
+    # Despite the name, also tests create and delete by necessity to have an update-able workspace.
+    # WARNING: This test takes a LONG time to run in live mode.
+    @pytest.mark.skipif(
+        condition=not is_live(),
+        reason="ARM template makes playback complex, so the test is flaky when run against recording",
+    )
+    def test_workspace_update(self, client: MLClient, randstr: Callable[[], str], location: str) -> None:
+        wps_name = f"e2etest_{randstr('wps_name')}"
+        wps_description = f"{wps_name} description"
+        wps_display_name = f"{wps_name} display name"
+        params_override = [
+            {"name": wps_name},
+            {"location": location},
+            {"description": wps_description},
+            {"display_name": wps_display_name},
+            {"enable_data_isolation": True},
+        ]
+
+        def workspace_validation(wps):
+            workspace_poller = client.workspaces.begin_create(workspace=wps)
+            assert isinstance(workspace_poller, LROPoller)
+            workspace = workspace_poller.result()
+            assert isinstance(workspace, Workspace)
+            assert workspace.name == wps_name
+            assert workspace.location == location
+            assert workspace.description == wps_description
+            assert workspace.display_name == wps_display_name
+            assert workspace.public_network_access == PublicNetworkAccess.ENABLED
+            # TODO uncomment this when enableDataIsolation flag change bug resolved for PATCH on the service side
+            # assert workspace.enable_data_isolation == True
+
+        workspace = verify_entity_load_and_dump(
+            load_workspace,
+            workspace_validation,
+            "./tests/test_configs/workspace/workspace_min.yaml",
+            params_override=params_override,
+        )[0]
+
+        workspace_list = client.workspaces.list()
+        assert isinstance(workspace_list, ItemPaged)
+        workspace = client.workspaces.get(wps_name)
+        assert isinstance(workspace, Workspace)
+        assert workspace.name == wps_name
+
         param_image_build_compute = "compute"
         param_display_name = "Test display name"
         param_description = "Test description"
@@ -90,8 +137,6 @@ def workspace_validation(wps):
             description=param_description,
             image_build_compute=param_image_build_compute,
             public_network_access=PublicNetworkAccess.DISABLED,
-            container_registry=static_acr,
-            application_insights=static_appinsights,
             update_dependent_resources=True,
             tags=param_tags,
         )
@@ -102,12 +147,68 @@ def workspace_validation(wps):
         assert workspace.description == param_description
         assert workspace.image_build_compute == param_image_build_compute
         assert workspace.public_network_access == PublicNetworkAccess.DISABLED
-        assert workspace.container_registry.lower() == static_acr.lower()
-        assert workspace.application_insights.lower() == static_appinsights.lower()
         assert workspace.tags == param_tags
         # enable_data_isolation flag can be only set at workspace creation stage, update for both put/patch is invliad
         # TODO uncomment this when enableDataIsolation flag change bug resolved for PATCH on the service side
         # assert workspace.enable_data_isolation == True
+        poller = client.workspaces.begin_delete(wps_name, delete_dependent_resources=True, permanently_delete=True)
+        # verify that request was accepted by checking if poller is returned
+        assert poller
+        assert isinstance(poller, LROPoller)
+
+    @pytest.mark.skipif(
+        condition=True,
+        reason="This test was refactored out from the original workspace CRUD test because not everyone has access to the "
+        + "static resources referenced here. We need to refactor the testing of ACR and appinsights to "
+        + "not be dependent on user access rights.",
+    )
+    def test_acr_and_appinsights_in_create(self, client: MLClient, randstr: Callable[[], str], location: str) -> None:
+        wps_name = f"e2etest_{randstr('wps_name')}"
+        wps_description = f"{wps_name} description"
+        wps_display_name = f"{wps_name} display name"
+        params_override = [
+            {"name": wps_name},
+            {"location": location},
+            {"description": wps_description},
+            {"display_name": wps_display_name},
+            {"enable_data_isolation": True},
+            {
+                "container_registry": "/subscriptions/8f338f6e-4fce-44ae-969c-fc7d8fda030e/resourceGroups/rg-mhe-e2e-test-dont-remove/providers/Microsoft.ContainerRegistry/registries/acrmhetest2"
+            },
+            {
+                "application_insights": "/subscriptions/8f338f6e-4fce-44ae-969c-fc7d8fda030e/resourceGroups/rg-mhe-e2e-test-dont-remove/providers/microsoft.insights/components/aimhetest2"
+            },
+        ]
+
+        # only test simple aspects of both a pointer and path-loaded workspace
+        # save actual service calls for a single object (below).
+        def workspace_validation(wps):
+            workspace_poller = client.workspaces.begin_create(workspace=wps)
+            assert isinstance(workspace_poller, LROPoller)
+            workspace = workspace_poller.result()
+            assert isinstance(workspace, Workspace)
+            assert workspace.name == wps_name
+            assert workspace.location == location
+            assert workspace.description == wps_description
+            assert workspace.display_name == wps_display_name
+            assert workspace.public_network_access == PublicNetworkAccess.ENABLED
+            # TODO uncomment this when enableDataIsolation flag change bug resolved for PATCH on the service side
+            # assert workspace.enable_data_isolation == True
+
+        workspace = verify_entity_load_and_dump(
+            load_workspace,
+            workspace_validation,
+            "./tests/test_configs/workspace/workspace_min.yaml",
+            params_override=params_override,
+        )[0]
+
+        workspace_list = client.workspaces.list()
+        assert isinstance(workspace_list, ItemPaged)
+        workspace = client.workspaces.get(wps_name)
+        assert isinstance(workspace, Workspace)
+        assert workspace.name == wps_name
+        assert workspace.container_registry.lower() == params_override["container_registry"].lower()
+        assert workspace.application_insights.lower() == params_override["application_insights"].lower()
 
         poller = client.workspaces.begin_delete(wps_name, delete_dependent_resources=True, permanently_delete=True)
         # verify that request was accepted by checking if poller is returned