fix: minor pr comments

Author: sagar-salvi-apptware

metadata-ingestion/src/datahub/ingestion/source/looker/looker_file_loader.py

@@ -1,7 +1,7 @@
 import logging
 import pathlib
 from dataclasses import replace
-from typing import Dict, List, Optional
+from typing import Dict, Optional
 
 from datahub.ingestion.source.looker.looker_config import LookerConnectionDefinition
 from datahub.ingestion.source.looker.looker_dataclasses import (
@@ -33,7 +33,7 @@ def __init__(
         base_projects_folder: Dict[str, pathlib.Path],
         reporter: LookMLSourceReport,
         source_config: LookMLSourceConfig,
-        manifest_constants: List[LookerConstant] = [],
+        manifest_constants: Dict[str, LookerConstant] = {},
     ) -> None:
         self.viewfile_cache: Dict[str, Optional[LookerViewFile]] = {}
         self._root_project_name = root_project_name

metadata-ingestion/src/datahub/ingestion/source/looker/looker_template_language.py

@@ -105,24 +105,18 @@ def resolve_liquid_variable(
         # Resolve liquid template
         return create_template(text).render(liquid_variable)
     except LiquidSyntaxError as e:
-        report.info(
-            title="Unsupported liquid template",
-            message="There are some tag specific to looker and python-liquid library does not understand them. Currently we are not parsing such liquid template.",
-            context=f"view-name: {view_name}, text: {text}, liquid_variable: {liquid_variable}",
-            exc=e,
-        )
+        # TODO: Will add warning once we get rid of duplcate warning message for same view
+        logger.warning(f"Unsupported liquid template encountered. error [{e.message}]")
         # TODO: There are some tag specific to looker and python-liquid library does not understand them. currently
         #  we are not parsing such liquid template.
         #
         # See doc: https://cloud.google.com/looker/docs/templated-filters and look for { % condition region %}
         # order.region { % endcondition %}
     except CustomTagException as e:
-        report.info(
-            title="Unsupported liquid template",
-            message="Rendering the Liquid template failed due to an issue with the provided variables or template syntax",
-            context=f"view-name: {view_name}, text: {text}, liquid_variable: {liquid_variable}",
-            exc=e,
-        )
+        # TODO: Will add warning once we get rid of duplcate warning message for same view
+        logger.warning(e)
+        logger.debug(e, exc_info=e)
+
     return text
 
 
@@ -370,7 +364,7 @@ def __init__(
         self,
         source_config: LookMLSourceConfig,
         reporter: LookMLSourceReport,
-        manifest_constants: List["LookerConstant"],
+        manifest_constants: Dict[str, "LookerConstant"],
     ):
         super().__init__(source_config=source_config, reporter=reporter)
         self.manifest_constants = manifest_constants
@@ -388,17 +382,8 @@ def replace_constants(match):
                 return str(self.source_config.lookml_constants.get(key))
 
             # Resolve constant from manifest
-            if self.manifest_constants:
-                value = next(
-                    (
-                        constant.value
-                        for constant in self.manifest_constants
-                        if constant.name == key
-                    ),
-                    None,
-                )
-                if value:
-                    return value
+            if key in self.manifest_constants:
+                return self.manifest_constants[key].value
 
             # Check if it's a misplaced lookml constant
             if key in self.source_config.liquid_variables:
@@ -479,7 +464,7 @@ def process_lookml_template_language(
     source_config: LookMLSourceConfig,
     view_lkml_file_dict: dict,
     reporter: LookMLSourceReport,
-    manifest_constants: List["LookerConstant"] = [],
+    manifest_constants: Dict[str, "LookerConstant"] = {},
     resolve_constants: bool = False,
 ) -> None:
     if "views" not in view_lkml_file_dict:
@@ -522,7 +507,7 @@ def load_and_preprocess_file(
     path: Union[str, pathlib.Path],
     source_config: LookMLSourceConfig,
     reporter: LookMLSourceReport,
-    manifest_constants: List["LookerConstant"] = [],
+    manifest_constants: Dict[str, "LookerConstant"] = {},
     resolve_constants: bool = False,
 ) -> dict:
     parsed = load_lkml(path)

metadata-ingestion/src/datahub/ingestion/source/looker/lookml_source.py

@@ -311,7 +311,7 @@ def __init__(self, config: LookMLSourceConfig, ctx: PipelineContext):
                     "manage_models permission enabled on this API key."
                 ) from err
 
-        self.manifest_constants: List[LookerConstant] = []
+        self.manifest_constants: Dict[str, "LookerConstant"] = {}
 
     def _load_model(self, path: str) -> LookerModel:
         logger.debug(f"Loading model from file {path}")
@@ -605,7 +605,7 @@ def _recursively_check_manifests(
         tmp_dir: str,
         project_name: str,
         project_visited: Set[str],
-        manifest_constants: List[LookerConstant],
+        manifest_constants: Dict[str, "LookerConstant"],
     ) -> None:
         if project_name in project_visited:
             return
@@ -623,14 +623,13 @@ def _recursively_check_manifests(
             return
 
         if manifest.constants:
-            manifest_constants.extend(
-                LookerConstant(
-                    name=constant["name"],
-                    value=constant["value"],
-                )
-                for constant in manifest.constants
-                if constant.get("name") and constant.get("value")
-            )
+            for constant in manifest.constants:
+                if constant.get("name") and constant.get("value"):
+                    manifest_constants[constant["name"]] = LookerConstant(
+                        name=constant["name"],
+                        value=constant["value"],
+                    )
+
         # Special case handling if the root project has a name in the manifest file.
         if project_name == BASE_PROJECT_NAME and manifest.project_name:
             if (
@@ -710,6 +709,7 @@ def get_internal_workunits(self) -> Iterable[MetadataWorkUnit]:  # noqa: C901
             self.source_config,
             self.manifest_constants,
         )
+        logger.debug(f"LookML Constants : {', '.join(self.manifest_constants.keys())}")
 
         # Some views can be mentioned by multiple 'include' statements and can be included via different connections.
 

metadata-ingestion/tests/integration/lookml/test_lookml.py

@@ -1083,10 +1083,10 @@ def test_lookml_constant_transformer(view, expected_result, warning_expected):
     transformer = LookmlConstantTransformer(
         source_config=config,
         reporter=report,
-        manifest_constants=[
-            LookerConstant(name="constant1", value="manifest_value1"),
-            LookerConstant(name="constant3", value="manifest_value3"),
-        ],
+        manifest_constants={
+            "constant1": LookerConstant(name="constant1", value="manifest_value1"),
+            "constant3": LookerConstant(name="constant3", value="manifest_value3"),
+        },
     )
 
     result = transformer.transform(view)