From fa75f62b6ee4a745261fbfa7e896d3a99f0cda03 Mon Sep 17 00:00:00 2001 From: janmatzek Date: Fri, 3 Jul 2026 11:46:55 +0200 Subject: [PATCH 1/2] fix: create placeholder metrics when Legacy MAQL cannot be translated to Cloud JIRA: SVS-1335 --- .../metrics/cloud_metric.py | 17 ++- src/gooddata_legacy2cloud/metrics/utils.py | 40 +++-- .../legacy_objects/objects_by_uri.json | 20 +++ .../with_unresolved_identifier_cloud.json | 17 +++ .../with_unresolved_identifier_legacy.json | 63 ++++++++ tests/test_metrics.py | 1 + tests/test_metrics_utils.py | 143 ++++++++++++++++++ 7 files changed, 285 insertions(+), 16 deletions(-) create mode 100644 tests/data/metrics/test_cases/with_unresolved_identifier_cloud.json create mode 100644 tests/data/metrics/test_cases/with_unresolved_identifier_legacy.json create mode 100644 tests/test_metrics_utils.py diff --git a/src/gooddata_legacy2cloud/metrics/cloud_metric.py b/src/gooddata_legacy2cloud/metrics/cloud_metric.py index 839c801..6b6276c 100644 --- a/src/gooddata_legacy2cloud/metrics/cloud_metric.py +++ b/src/gooddata_legacy2cloud/metrics/cloud_metric.py @@ -10,7 +10,10 @@ from gooddata_legacy2cloud.helpers import get_cloud_id, parse_legacy_tags from gooddata_legacy2cloud.metrics.data_classes import MetricContext from gooddata_legacy2cloud.metrics.cloud_maql import CloudMaql -from gooddata_legacy2cloud.metrics.utils import get_folders_names +from gooddata_legacy2cloud.metrics.utils import ( + build_placeholder_maql, + get_folders_names, +) logger = logging.getLogger("migration") @@ -32,6 +35,7 @@ def __init__( self.legacy_maql = self.metric_content["expression"] self.description = metadata["metric"]["meta"]["summary"] self.tags = self._get_tags() + self.has_error = False self.cloud_maql, self.errors = self._get_cloud_maql( self.legacy_maql, self.metric_content["tree"] ) @@ -86,7 +90,11 @@ def _get_cloud_maql(self, legacy_maql: str, content_tree: Any): logger.warning("Metric '%s': %s", self.meta["title"], maql_errors) except Exception as e: - raise Exception(f"ERROR `{self.meta['title']}`: {e}") from e + # If transformation fails, create an error placeholder + self.has_error = True + new_maql = build_placeholder_maql(legacy_maql, str(e)) + maql_errors = [str(e)] + logger.error("Metric '%s': %s", self.meta["title"], e) return new_maql, maql_errors @@ -95,7 +103,10 @@ def get(self): Returns the Cloud metric object. """ - if self.get_errors() and not self.ctx.suppress_warnings: + if self.has_error: + self.tags.append("ERROR") + self.meta["cloud_title"] = f"[ERROR] {self.meta['title']}" + elif self.get_errors() and not self.ctx.suppress_warnings: self.meta["cloud_title"] = f"[WARN] {self.meta['title']}" return { diff --git a/src/gooddata_legacy2cloud/metrics/utils.py b/src/gooddata_legacy2cloud/metrics/utils.py index 509d76e..02a72b3 100644 --- a/src/gooddata_legacy2cloud/metrics/utils.py +++ b/src/gooddata_legacy2cloud/metrics/utils.py @@ -11,7 +11,7 @@ from gooddata_legacy2cloud.metrics.contants import DAY_SHORTCUTS -def _comment_out_lines(text: str) -> str: +def comment_out_lines(text: str) -> str: """Helper function to comment out all lines in a text string.""" return "\n".join(f"#{line}" for line in text.splitlines()) @@ -41,6 +41,22 @@ def adjust_comment_for_broken_metric(output): return output +def build_placeholder_maql( + original_maql: str, + extra_message: str | None = None, + error_label: str | None = "Error", +) -> str: + """ + Comments out the original MAQL, optionally appends a labeled note, and adds a + SQRT(-1) placeholder expression so the metric can still be created in Cloud. + """ + commented_maql = adjust_comment_for_broken_metric(comment_out_lines(original_maql)) + if extra_message: + label = f"#{error_label}:\n" if error_label else "" + commented_maql += f"\n\n{label}{comment_out_lines(extra_message)}" + return f"#Failed MAQL:\n{commented_maql}\n\nSELECT SQRT(-1)" + + def disable_broken_metric(metric, error_response=None): """ Fixes a broken metric by adding a tag and commenting out the original MAQL. @@ -59,29 +75,27 @@ def disable_broken_metric(metric, error_response=None): attribute["title"] = f"[ERROR] {attribute['title']}" maql = metric["data"]["attributes"]["content"]["maql"] - commented_maql = _comment_out_lines(maql) - commented_maql = adjust_comment_for_broken_metric(commented_maql) - # Add API error response if provided + error_message = "" + extra_label = None if error_response is not None: # Extract the error text (either detail field or full response) - error_message = "" if hasattr(error_response, "text") and error_response.text: try: error_data = json.loads(error_response.text) - error_message = error_data.get("detail", error_response.text) + error_message = ( + error_data.get("detail", error_response.text) + if isinstance(error_data, dict) + else error_response.text + ) except json.JSONDecodeError: error_message = error_response.text - # Add the commented error to the MAQL if error_message: - commented_error = _comment_out_lines(error_message) - commented_maql += ( - f"\n\n#API Error {error_response.status_code}:\n{commented_error}" - ) + extra_label = f"API Error {error_response.status_code}" - metric["data"]["attributes"]["content"]["maql"] = ( - f"#Failed MAQL:\n{commented_maql}\n\nSELECT SQRT(-1)" + metric["data"]["attributes"]["content"]["maql"] = build_placeholder_maql( + maql, error_message, extra_label ) return metric diff --git a/tests/data/metrics/legacy_objects/objects_by_uri.json b/tests/data/metrics/legacy_objects/objects_by_uri.json index 06c5a13..0956ad0 100644 --- a/tests/data/metrics/legacy_objects/objects_by_uri.json +++ b/tests/data/metrics/legacy_objects/objects_by_uri.json @@ -1,4 +1,24 @@ { + "/gdc/md/dinu996plcf6ajcp6wygnqn952uyom9u/obj/9001": { + "attribute": { + "content": { + "displayForms": [ + { + "links": { + "elements": "/gdc/md/dinu996plcf6ajcp6wygnqn952uyom9u/obj/9002/elements" + }, + "meta": { + "identifier": "attr.comp.computedAttr" + } + } + ] + }, + "meta": { + "category": "attribute", + "identifier": "attr.comp.computedAttr" + } + } + }, "/gdc/md/dinu996plcf6ajcp6wygnqn952uyom9u/obj/4769": { "fact": { "content": { diff --git a/tests/data/metrics/test_cases/with_unresolved_identifier_cloud.json b/tests/data/metrics/test_cases/with_unresolved_identifier_cloud.json new file mode 100644 index 0000000..9b51642 --- /dev/null +++ b/tests/data/metrics/test_cases/with_unresolved_identifier_cloud.json @@ -0,0 +1,17 @@ +[ + { + "data": { + "id": "metric_with_computed_attribute_aaCompAttrMetric1", + "type": "metric", + "attributes": { + "title": "[ERROR] Metric with computed attribute", + "tags": ["ERROR"], + "description": "", + "content": { + "format": "#,##0.00", + "maql": "#Failed MAQL:\n#SELECT COUNT([/gdc/md/dinu996plcf6ajcp6wygnqn952uyom9u/obj/9001])\n\n#Error:\n#Search Cloud Id - Unknown Legacy identifier attr.comp.computedAttr\n\nSELECT SQRT(-1)" + } + } + } + } +] diff --git a/tests/data/metrics/test_cases/with_unresolved_identifier_legacy.json b/tests/data/metrics/test_cases/with_unresolved_identifier_legacy.json new file mode 100644 index 0000000..ca057c1 --- /dev/null +++ b/tests/data/metrics/test_cases/with_unresolved_identifier_legacy.json @@ -0,0 +1,63 @@ +[ + { + "metric": { + "content": { + "format": "#,##0.00", + "tree": { + "position": { + "line": 1, + "column": 14 + }, + "type": "metric", + "content": [ + { + "content": [ + { + "type": "function", + "value": "COUNT", + "position": { + "column": 21, + "line": 1 + }, + "content": [ + { + "value": "/gdc/md/dinu996plcf6ajcp6wygnqn952uyom9u/obj/9001", + "type": "attribute object", + "position": { + "column": 28, + "line": 1 + } + } + ] + } + ], + "type": "expression", + "position": { + "column": 21, + "line": 1 + } + } + ] + }, + "expression": "SELECT COUNT([/gdc/md/dinu996plcf6ajcp6wygnqn952uyom9u/obj/9001])" + }, + "meta": { + "isProduction": 1, + "summary": "", + "contributor": "/gdc/account/profile/5cc081c981561ae1d3f81481b50f002c", + "updated": "2025-04-10 09:26:53", + "identifier": "aaCompAttrMetric1", + "tags": "", + "uri": "/gdc/md/dinu996plcf6ajcp6wygnqn952uyom9u/obj/9010", + "title": "Metric with computed attribute", + "created": "2025-04-10 09:24:10", + "category": "metric", + "deprecated": "0", + "author": "/gdc/account/profile/5cc081c981561ae1d3f81481b50f002c" + }, + "links": { + "explain2": "/gdc/md/dinu996plcf6ajcp6wygnqn952uyom9u/obj/9010/explain2" + } + } + } +] diff --git a/tests/test_metrics.py b/tests/test_metrics.py index cec640b..d955111 100644 --- a/tests/test_metrics.py +++ b/tests/test_metrics.py @@ -24,6 +24,7 @@ "with_nonexistent_value_in_filter", "with_date_filters", "using_deprecated_metric", + "with_unresolved_identifier", ], ) def test_metrics_migration( diff --git a/tests/test_metrics_utils.py b/tests/test_metrics_utils.py new file mode 100644 index 0000000..0a39704 --- /dev/null +++ b/tests/test_metrics_utils.py @@ -0,0 +1,143 @@ +# (C) 2026 GoodData Corporation +""" +Unit tests for gooddata_legacy2cloud.metrics.utils helper functions. +""" + +import json +from types import SimpleNamespace + +from gooddata_legacy2cloud.metrics.utils import ( + build_placeholder_maql, + comment_out_lines, + disable_broken_metric, +) + + +def test_comment_out_lines_prefixes_every_line(): + assert comment_out_lines("first line\nsecond line") == "#first line\n#second line" + + +def test_build_placeholder_maql_without_extra_message(): + result = build_placeholder_maql("SELECT COUNT([/gdc/md/project/obj/1])") + + assert result == ( + "#Failed MAQL:\n#SELECT COUNT([/gdc/md/project/obj/1])\n\nSELECT SQRT(-1)" + ) + + +def test_build_placeholder_maql_defaults_to_error_label(): + result = build_placeholder_maql( + "SELECT SUM([/gdc/md/project/obj/2])", "something went wrong" + ) + + assert result == ( + "#Failed MAQL:\n#SELECT SUM([/gdc/md/project/obj/2])" + "\n\n#Error:\n#something went wrong\n\nSELECT SQRT(-1)" + ) + + +def test_build_placeholder_maql_with_custom_label(): + result = build_placeholder_maql( + "SELECT SUM([/gdc/md/project/obj/3])", + "Cannot resolve reference", + "API Error 400", + ) + + assert result == ( + "#Failed MAQL:\n#SELECT SUM([/gdc/md/project/obj/3])" + "\n\n#API Error 400:\n#Cannot resolve reference\n\nSELECT SQRT(-1)" + ) + + +def test_build_placeholder_maql_with_explicit_no_label(): + result = build_placeholder_maql( + "SELECT SUM([/gdc/md/project/obj/4])", "first line\nsecond line", None + ) + + assert result == ( + "#Failed MAQL:\n#SELECT SUM([/gdc/md/project/obj/4])" + "\n\n#first line\n#second line\n\nSELECT SQRT(-1)" + ) + + +def test_disable_broken_metric_without_error_response(): + metric = { + "data": { + "attributes": { + "title": "My metric", + "tags": ["existing"], + "content": {"maql": "SELECT SUM({fact/orders.amount})"}, + } + } + } + + result = disable_broken_metric(metric) + + attributes = result["data"]["attributes"] + assert attributes["title"] == "[ERROR] My metric" + assert attributes["tags"] == ["existing", "ERROR"] + assert attributes["content"]["maql"] == ( + "#Failed MAQL:\n#SELECT SUM({fact/orders.amount})\n\nSELECT SQRT(-1)" + ) + + +def test_disable_broken_metric_with_no_existing_tags(): + metric = { + "data": { + "attributes": { + "title": "My metric", + "tags": None, + "content": {"maql": "SELECT SUM({fact/orders.amount})"}, + } + } + } + + result = disable_broken_metric(metric) + + assert result["data"]["attributes"]["tags"] == ["ERROR"] + + +def test_disable_broken_metric_appends_api_error_detail(): + metric = { + "data": { + "attributes": { + "title": "My metric", + "tags": [], + "content": {"maql": "SELECT SUM({fact/orders.amount})"}, + } + } + } + error_response = SimpleNamespace( + status_code=400, + text=json.dumps({"detail": "Cannot resolve reference"}), + ) + + result = disable_broken_metric(metric, error_response) + + assert result["data"]["attributes"]["content"]["maql"] == ( + "#Failed MAQL:\n#SELECT SUM({fact/orders.amount})" + "\n\n#API Error 400:\n#Cannot resolve reference\n\nSELECT SQRT(-1)" + ) + + +def test_disable_broken_metric_with_non_object_json_error_body(): + metric = { + "data": { + "attributes": { + "title": "My metric", + "tags": [], + "content": {"maql": "SELECT SUM({fact/orders.amount})"}, + } + } + } + error_response = SimpleNamespace( + status_code=500, + text=json.dumps(["Internal Server Error"]), + ) + + result = disable_broken_metric(metric, error_response) + + assert result["data"]["attributes"]["content"]["maql"] == ( + "#Failed MAQL:\n#SELECT SUM({fact/orders.amount})" + '\n\n#API Error 500:\n#["Internal Server Error"]\n\nSELECT SQRT(-1)' + ) From 061971540570a186dd7298c082af709acbb34a77 Mon Sep 17 00:00:00 2001 From: janmatzek Date: Fri, 3 Jul 2026 12:04:41 +0200 Subject: [PATCH 2/2] chore: update copyright header AI instructions JIRA: trivial --- .claude/rules/coding_standards.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.claude/rules/coding_standards.md b/.claude/rules/coding_standards.md index c8ca048..e63a7e4 100644 --- a/.claude/rules/coding_standards.md +++ b/.claude/rules/coding_standards.md @@ -4,9 +4,11 @@ First line of every new file: ```python -# (C) 2025 GoodData Corporation +# (C) {year} GoodData Corporation ``` +where `{year}` represents current year. + ## Python Version & Style - Target: Python 3.14