PR feedback

Author: georgesittas

sqlmesh/core/model/common.py

@@ -440,6 +440,19 @@ def _executable_to_str(k: str, v: Executable) -> str:
     return [_executable_to_str(k, v) for k, v in sort_python_env(python_env)]
 
 
+def parse_strings_with_macro_refs(value: t.Any, dialect: DialectType) -> t.Any:
+    if isinstance(value, str) and "@" in value:
+        return exp.maybe_parse(value, dialect=dialect)
+
+    if isinstance(value, dict):
+        for k, v in dict(value).items():
+            value[k] = parse_strings_with_macro_refs(v, dialect)
+    elif isinstance(value, list):
+        value = [parse_strings_with_macro_refs(v, dialect) for v in value]
+
+    return value
+
+
 expression_validator: t.Callable = field_validator(
     "query",
     "expressions_",

sqlmesh/core/model/definition.py

@@ -27,6 +27,7 @@
     expression_validator,
     make_python_env,
     parse_dependencies,
+    parse_strings_with_macro_refs,
     single_value_or_tuple,
     sorted_python_env_payloads,
     validate_extra_and_required_fields,
@@ -72,6 +73,9 @@
 
 logger = logging.getLogger(__name__)
 
+
+UNRENDERABLE_MODEL_FIELDS = {"cron", "description"}
+
 PROPERTIES = {"physical_properties", "session_properties", "virtual_properties"}
 
 RUNTIME_RENDERED_MODEL_FIELDS = {
@@ -2750,34 +2754,23 @@ def render_field_value(value: t.Any) -> t.Any:
 
         return value
 
-    def parse_strings_with_macro_refs(value: t.Any) -> t.Any:
-        if isinstance(value, str) and "@" in value:
-            return exp.maybe_parse(value, dialect=dialect)
-
-        if isinstance(value, dict):
-            for k, v in dict(value).items():
-                value[k] = parse_strings_with_macro_refs(v)
-        elif isinstance(value, list):
-            value = [parse_strings_with_macro_refs(v) for v in value]
-
-        return value
-
     for field_name, field_info in ModelMeta.all_field_infos().items():
         field = field_info.alias or field_name
         field_value = fields.get(field)
 
-        if field in ("cron", "description") or field_value is None:
+        # We don't want to parse python model cron="@..." kwargs (e.g. @daily) into MacroVar
+        if field == "cron" or field_value is None:
             continue
 
         if field in RUNTIME_RENDERED_MODEL_FIELDS:
-            fields[field] = parse_strings_with_macro_refs(field_value)
+            fields[field] = parse_strings_with_macro_refs(field_value, dialect)
             continue
 
         if isinstance(field_value, dict):
             rendered_dict = {}
             for key, value in field_value.items():
                 if key in RUNTIME_RENDERED_MODEL_FIELDS:
-                    rendered_dict[key] = parse_strings_with_macro_refs(value)
+                    rendered_dict[key] = parse_strings_with_macro_refs(value, dialect)
                 elif (rendered := render_field_value(value)) is not None:
                     rendered_dict[key] = rendered
 

tests/core/test_model.py

@@ -6436,7 +6436,7 @@ def end_date_macro(evaluator: MacroEvaluator, var: bool):
         owner="@IF(@gateway = 'dev', @{dev_owner}, @{prod_owner})",
         stamp="@{stamp}",
         tags=["@{tag1}", "@{tag2}"],
-        description="Model desc @{test_}",
+        description="'Model desc @{test_}'",
     )
     def model_with_macros(evaluator, **kwargs):
         return exp.select(