Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .claude/rules/coding_standards.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 14 additions & 3 deletions src/gooddata_legacy2cloud/metrics/cloud_metric.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand All @@ -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"]
)
Expand Down Expand Up @@ -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

Expand All @@ -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 {
Expand Down
40 changes: 27 additions & 13 deletions src/gooddata_legacy2cloud/metrics/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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())

Expand Down Expand Up @@ -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.
Expand All @@ -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
Comment thread
coderabbitai[bot] marked this conversation as resolved.
)
return metric

Expand Down
20 changes: 20 additions & 0 deletions tests/data/metrics/legacy_objects/objects_by_uri.json
Original file line number Diff line number Diff line change
@@ -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": {
Expand Down
Original file line number Diff line number Diff line change
@@ -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)"
}
}
}
}
]
Original file line number Diff line number Diff line change
@@ -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"
}
}
}
]
1 change: 1 addition & 0 deletions tests/test_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
"with_nonexistent_value_in_filter",
"with_date_filters",
"using_deprecated_metric",
"with_unresolved_identifier",
],
)
def test_metrics_migration(
Expand Down
143 changes: 143 additions & 0 deletions tests/test_metrics_utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
# (C) 2026 GoodData Corporation
Comment thread
coderabbitai[bot] marked this conversation as resolved.
"""
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)'
)
Loading