MINOR: Fix e2e (#16627)

* Fix Metabase E2E Test * Add 'debug' input to python e2e tests * Fix 'debug' default to be 'false' * Standardized all Metabase IDs to MetabaseStrId * Fix Metabase expected filtered sink mix value * Fix wrong parameter being passed to the config * Fix powerBI e2e tests * Fix one Redash e2e test * Fix checkstyle * Fix Dashboard create patch_request not using EntityReferenceList * Fix Redash E2E test value * Add logging to create patch request * Fix checkstyle and linting * Fix default debug value * Fix e2e workflow * Fix e2e workflow * Fix e2e workflow * Fix metabase and powerbi e2e values
open-metadata · Jun 12, 2024 · 328ed2b · 328ed2b
1 parent eb3b1cb
commit 328ed2b
Show file tree

Hide file tree

Showing 3 changed files with 85 additions and 57 deletions.
diff --git a/.github/workflows/py-cli-e2e-tests.yml b/.github/workflows/py-cli-e2e-tests.yml
@@ -24,11 +24,22 @@ on:
         required: False
         default: "false"
 
+env:
+  DEBUG: ${{ inputs.debug || 'false' }}
+
 permissions:
   id-token: write
   contents: read
 
 jobs:
+  # Needed since env is not available on the job context: https://github.com/actions/runner/issues/2372
+  check-debug:
+    runs-on: ubuntu-latest
+    outputs:
+      DEBUG: ${{ env.DEBUG }}
+    steps:
+      - run: echo "null"
+
   py-cli-e2e-tests:
     runs-on: ubuntu-latest
     strategy:
@@ -152,21 +163,21 @@ jobs:
         coverage report --rcfile ingestion/pyproject.toml --data-file .coverage.$E2E_TEST || true
 
     - name: Upload coverage artifact for Python tests
-      if: matrix.e2e-test == 'python' && steps.python-e2e-test.outcome == 'success' && inputs.debug == 'false'
+      if: matrix.e2e-test == 'python' && steps.python-e2e-test.outcome == 'success' && env.DEBUG == 'false'
       uses: actions/upload-artifact@v3
       with:
         name: coverage-${{ matrix.e2e-test }}
         path: .coverage
 
     - name: Upload coverage artifact for CLI E2E tests
-      if: matrix.e2e-test != 'python' && steps.e2e-test.outcome == 'success' && inputs.debug == 'false'
+      if: matrix.e2e-test != 'python' && steps.e2e-test.outcome == 'success' && env.DEBUG == 'false'
       uses: actions/upload-artifact@v3
       with:
         name: coverage-${{ matrix.e2e-test }}
         path: .coverage.${{ matrix.e2e-test }}
 
     - name: Upload tests artifact
-      if: steps.e2e-test.outcome == 'success' || steps.python-e2e-test.outcome == 'success' && inputs.debug == 'false'
+      if: steps.e2e-test.outcome == 'success' || steps.python-e2e-test.outcome == 'success' && env.DEBUG == 'false'
       uses: actions/upload-artifact@v3
       with:
         name: tests-${{ matrix.e2e-test }}
@@ -179,7 +190,7 @@ jobs:
         sudo rm -rf ${PWD}/docker-volume
 
     - name: Slack on Failure
-      if: steps.e2e-test.outcome != 'success' && steps.python-e2e-test.outcome != 'success' && inputs.debug == 'false'
+      if: steps.e2e-test.outcome != 'success' && steps.python-e2e-test.outcome != 'success' && env.DEBUG == 'false'
       uses: slackapi/slack-github-action@v1.23.0
       with:
         payload: |
@@ -197,8 +208,10 @@ jobs:
 
   sonar-cloud-coverage-upload:
     runs-on: ubuntu-latest
-    needs: py-cli-e2e-tests
-    if: inputs.debug == 'false'
+    needs:
+      - py-cli-e2e-tests
+      - check-debug
+    if: needs.check-debug.outputs.DEBUG == 'false'
     steps:
 
       - name: Checkout

diff --git a/ingestion/src/metadata/ingestion/models/patch_request.py b/ingestion/src/metadata/ingestion/models/patch_request.py
@@ -12,6 +12,8 @@
 Pydantic definition for storing entities for patching
 """
 import json
+import logging
+import traceback
 from typing import Dict, List, Optional, Tuple
 
 import jsonpatch
@@ -21,6 +23,8 @@
 from metadata.ingestion.ometa.mixins.patch_mixin_utils import PatchOperation
 from metadata.ingestion.ometa.utils import model_str
 
+logger = logging.getLogger("metadata")
+
 
 class PatchRequest(BaseModel):
     """
@@ -326,57 +330,63 @@ def build_patch(
     Returns
         Updated Entity
     """
-
-    # remove change descriptions from entities
-    if remove_change_description:
-        source = _remove_change_description(source)
-        destination = _remove_change_description(destination)
-
-    if array_entity_fields:
-        _sort_array_entity_fields(
-            source=source,
-            destination=destination,
-            array_entity_fields=array_entity_fields,
-        )
-
-    # Get the difference between source and destination
-    if allowed_fields:
-        patch = jsonpatch.make_patch(
-            json.loads(
-                source.model_dump_json(
-                    exclude_unset=True,
-                    exclude_none=True,
-                    include=allowed_fields,
-                )
-            ),
-            json.loads(
-                destination.model_dump_json(
-                    exclude_unset=True,
-                    exclude_none=True,
-                    include=allowed_fields,
-                )
-            ),
-        )
-    else:
-        patch: jsonpatch.JsonPatch = jsonpatch.make_patch(
-            json.loads(source.model_dump_json(exclude_unset=True, exclude_none=True)),
-            json.loads(
-                destination.model_dump_json(exclude_unset=True, exclude_none=True)
-            ),
-        )
-    if not patch:
+    try:
+        # remove change descriptions from entities
+        if remove_change_description:
+            source = _remove_change_description(source)
+            destination = _remove_change_description(destination)
+
+        if array_entity_fields:
+            _sort_array_entity_fields(
+                source=source,
+                destination=destination,
+                array_entity_fields=array_entity_fields,
+            )
+
+        # Get the difference between source and destination
+        if allowed_fields:
+            patch = jsonpatch.make_patch(
+                json.loads(
+                    source.model_dump_json(
+                        exclude_unset=True,
+                        exclude_none=True,
+                        include=allowed_fields,
+                    )
+                ),
+                json.loads(
+                    destination.model_dump_json(
+                        exclude_unset=True,
+                        exclude_none=True,
+                        include=allowed_fields,
+                    )
+                ),
+            )
+        else:
+            patch: jsonpatch.JsonPatch = jsonpatch.make_patch(
+                json.loads(
+                    source.model_dump_json(exclude_unset=True, exclude_none=True)
+                ),
+                json.loads(
+                    destination.model_dump_json(exclude_unset=True, exclude_none=True)
+                ),
+            )
+        if not patch:
+            return None
+
+        # For a user editable fields like descriptions, tags we only want to support "add" operation in patch
+        # we will remove the other operations.
+        if restrict_update_fields:
+            updated_operations = JsonPatchUpdater.from_restrict_update_fields(
+                restrict_update_fields
+            ).update(patch)
+            patch.patch = updated_operations
+
+        return patch
+    except Exception:
+        logger.debug(traceback.format_exc())
+        logger.warning("Couldn't build patch for Entity.")
         return None
 
-    # For a user editable fields like descriptions, tags we only want to support "add" operation in patch
-    # we will remove the other operations.
-    if restrict_update_fields:
-        updated_operations = JsonPatchUpdater.from_restrict_update_fields(
-            restrict_update_fields
-        ).update(patch)
-        patch.patch = updated_operations
-
-    return patch
-
 
 def _sort_array_entity_fields(
     source: T,

diff --git a/ingestion/src/metadata/ingestion/source/dashboard/dashboard_service.py b/ingestion/src/metadata/ingestion/source/dashboard/dashboard_service.py
@@ -49,6 +49,7 @@
 )
 from metadata.generated.schema.type.entityLineage import Source as LineageSource
 from metadata.generated.schema.type.entityReference import EntityReference
+from metadata.generated.schema.type.entityReferenceList import EntityReferenceList
 from metadata.generated.schema.type.usageRequest import UsageRequest
 from metadata.ingestion.api.delete import delete_entity_from_source
 from metadata.ingestion.api.models import Either, Entity
@@ -621,7 +622,9 @@ def create_patch_request(
                             type=LINEAGE_MAP[type(chart_entity)],
                         )
                     )
-            patch_request.new_entity.charts = charts_entity_ref_list
+            patch_request.new_entity.charts = EntityReferenceList(
+                charts_entity_ref_list
+            )
 
             # For patch the datamodels need to be entity ref instead of fqn
             datamodel_entity_ref_list = []
@@ -636,7 +639,9 @@ def create_patch_request(
                             type=LINEAGE_MAP[type(datamodel_entity)],
                         )
                     )
-            patch_request.new_entity.dataModels = datamodel_entity_ref_list
+            patch_request.new_entity.dataModels = EntityReferenceList(
+                datamodel_entity_ref_list
+            )
         return patch_request
 
     def _get_column_lineage(