Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ingestion/lookml): resolve access notation for LookML Constant #12277

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
15 changes: 15 additions & 0 deletions metadata-ingestion/docs/sources/looker/looker_recipe.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,19 @@ source:
client_id: ${LOOKER_CLIENT_ID}
client_secret: ${LOOKER_CLIENT_SECRET}

# Liquid variables
# liquid_variable:
sagar-salvi-apptware marked this conversation as resolved.
Show resolved Hide resolved
# _user_attributes:
# looker_env: "dev"
# dev_database_prefix: "employee"
# dev_schema_prefix: "public"
# dw_eff_dt_date:
# _is_selected: true
# source_region: "ap-south-1"
# db: "test-db"

# LookML Constant
# lookml_constant:
# star_award_winner_year: "public.winner_2025"

# sink configs
30 changes: 25 additions & 5 deletions metadata-ingestion/docs/sources/looker/lookml_post.md
Original file line number Diff line number Diff line change
@@ -1,11 +1,31 @@
#### Configuration Notes

1. If a view contains a liquid template (e.g. `sql_table_name: {{ user_attributes['db']}}.kafka_streaming.events }}`, with `db=ANALYTICS_PROD`), then you will need to specify the values of those variables in the `liquid_variable` config as shown below:
```yml
liquid_variable:
user_attributes:
db: ANALYTICS_PROD
```

```yml
liquid_variable:
user_attributes:
db: ANALYTICS_PROD
```

2. If a view contains a LookML constant (e.g., `sql_table_name: @{db}.kafka_streaming.events;`), its value will be resolved by first checking the manifest file and, if not found, then checking the config as shown below:.
sagar-salvi-apptware marked this conversation as resolved.
Show resolved Hide resolved

```yml
lookml_constant:
db: ANALYTICS_PROD
```

**Example**:

```lkml
sql_table_name: @{db}.kafka_streaming.events;;
```

**Resolved Output**:

```
ANALYTICS_PROD.kafka_streaming.events
```

### Multi-Project LookML (Advanced)

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import logging
import pathlib
from dataclasses import replace
from typing import Dict, Optional
from typing import Dict, List, Optional

from datahub.ingestion.source.looker.looker_config import LookerConnectionDefinition
from datahub.ingestion.source.looker.looker_dataclasses import LookerViewFile
Expand Down Expand Up @@ -30,12 +30,14 @@ def __init__(
base_projects_folder: Dict[str, pathlib.Path],
reporter: LookMLSourceReport,
source_config: LookMLSourceConfig,
looker_constant: Optional[List[Dict[str, str]]] = [],
) -> None:
self.viewfile_cache: Dict[str, Optional[LookerViewFile]] = {}
self._root_project_name = root_project_name
self._base_projects_folder = base_projects_folder
self.reporter = reporter
self.source_config = source_config
self.looker_constant = looker_constant

def _load_viewfile(
self, project_name: str, path: str, reporter: LookMLSourceReport
Expand Down Expand Up @@ -74,6 +76,7 @@ def _load_viewfile(
parsed = load_and_preprocess_file(
path=path,
source_config=self.source_config,
looker_constant=self.looker_constant,
)

looker_viewfile = LookerViewFile.from_looker_dict(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,15 +192,20 @@

source_config: LookMLSourceConfig

def __init__(self, source_config: LookMLSourceConfig):
def __init__(
self,
source_config: LookMLSourceConfig,
looker_constant: Optional[List[Dict[str, str]]] = [],
):
self.source_config = source_config
self.looker_constant = looker_constant

def transform(self, view: dict) -> dict:
value_to_transform: Optional[str] = None

# is_attribute_supported check is required because not all transformer works on all attributes in current
# case mostly all transformer works on sql_table_name and derived.sql attributes,
# however IncompleteSqlTransformer only transform the derived.sql attribute
# is_attribute_supported check is required because not all transformers work on all attributes in the current
# case, mostly all transformers work on sql_table_name and derived.sql attributes;
# however, IncompleteSqlTransformer only transform the derived.sql attribute
if SQL_TABLE_NAME in view and self.is_attribute_supported(SQL_TABLE_NAME):
# Give precedence to already processed transformed view.sql_table_name to apply more transformation
value_to_transform = view.get(
Expand Down Expand Up @@ -287,7 +292,7 @@

class DropDerivedViewPatternTransformer(LookMLViewTransformer):
"""
drop ${} from datahub_transformed_sql_table_name and view["derived_table"]["datahub_transformed_sql_table_name"] values.
drop ${} from datahub_transformed_sql_table_name and view["derived_table"]["datahub_transformed_sql_table_name"] values.

Example: transform ${employee_income_source.SQL_TABLE_NAME} to employee_income_source.SQL_TABLE_NAME
"""
Expand Down Expand Up @@ -335,6 +340,54 @@
return self._apply_regx(value)


class LookmlConstantTransformer(LookMLViewTransformer):
"""
Replace LookML constants @{constant} from the manifest/configuration.
"""

CONSTANT_PATTERN = r"@{(\w+)}" # Matches @{constant}

def resolve_lookml_parameter(self, text: str) -> str:
"""
Resolves LookML constants (@{ }) from manifest or config.
Logs warnings for misplaced or missing variables.
"""

def replace_constants(match):
key = match.group(1)
if self.looker_constant:
value = next(
(
item["value"]
for item in self.looker_constant
if item["name"] == key
),
None,
)
if value:
return value

# Resolve constant
if key in self.source_config.lookml_constant:
return str(self.source_config.lookml_constant.get(key))

# Check if it's a misplaced liquid variable
if key in self.source_config.liquid_variable:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should check the lookml_parameter field first

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was not done - lookml_constants should take precedence over the manifest file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is old code in newer commits i have removed it already

logger.warning(

Check warning on line 376 in metadata-ingestion/src/datahub/ingestion/source/looker/looker_template_language.py

View check run for this annotation

Codecov / codecov/patch

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

Added line #L376 was not covered by tests
f"Misplaced liquid variable '@{{{key}}}' detected. Use '${{{key}}}' instead."
sagar-salvi-apptware marked this conversation as resolved.
Show resolved Hide resolved
sagar-salvi-apptware marked this conversation as resolved.
Show resolved Hide resolved
)
return f"@{{{key}}}"

Check warning on line 379 in metadata-ingestion/src/datahub/ingestion/source/looker/looker_template_language.py

View check run for this annotation

Codecov / codecov/patch

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

Added line #L379 was not covered by tests
sagar-salvi-apptware marked this conversation as resolved.
Show resolved Hide resolved

logger.warning(f"Parameter '@{{{key}}}' not found in configuration.")

Check warning on line 381 in metadata-ingestion/src/datahub/ingestion/source/looker/looker_template_language.py

View check run for this annotation

Codecov / codecov/patch

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

Added line #L381 was not covered by tests
return "NULL"

# Resolve @{} (constant)
return re.sub(self.CONSTANT_PATTERN, replace_constants, text)

def _apply_transformation(self, value: str, view: dict) -> str:
return self.resolve_lookml_parameter(text=value)


class TransformedLookMlView:
"""
TransformedLookMlView is collecting output of LookMLViewTransformer and creating a new transformed LookML view.
Expand Down Expand Up @@ -390,6 +443,7 @@
def process_lookml_template_language(
source_config: LookMLSourceConfig,
view_lkml_file_dict: dict,
looker_constant: Optional[List[Dict[str, str]]],
) -> None:
if "views" not in view_lkml_file_dict:
return
Expand All @@ -401,6 +455,9 @@
LiquidVariableTransformer(
source_config=source_config
), # Now resolve liquid variables
LookmlConstantTransformer(
source_config=source_config, looker_constant=looker_constant
), # Resolve @{} constant with its corresponding value
DropDerivedViewPatternTransformer(
source_config=source_config
), # Remove any ${} symbol
Expand All @@ -422,12 +479,14 @@
def load_and_preprocess_file(
path: Union[str, pathlib.Path],
source_config: LookMLSourceConfig,
looker_constant: Optional[List[Dict[str, str]]] = [],
) -> dict:
parsed = load_lkml(path)

process_lookml_template_language(
view_lkml_file_dict=parsed,
source_config=source_config,
looker_constant=looker_constant,
)

return parsed
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,20 @@ class LookMLSourceConfig(

liquid_variable: Dict[Any, Any] = Field(
{},
description="A dictionary containing Liquid variables and their corresponding values, utilized in SQL-defined "
description="A dictionary containing Liquid variables, Liquid logic, and LookML parameters with their corresponding values, utilized in SQL-defined "
"derived views. The Liquid template will be resolved in view.derived_table.sql and "
"view.sql_table_name. Defaults to an empty dictionary.",
)

lookml_constant: Dict[str, str] = Field(
sagar-salvi-apptware marked this conversation as resolved.
Show resolved Hide resolved
{},
description=(
"A dictionary containing LookML constant (`@{constant_name}`) and their values. "
"Parameters are used to define dynamic references in LookML views, such as `view.sql_table_name`."
"Defaults to an empty dictionary."
sagar-salvi-apptware marked this conversation as resolved.
Show resolved Hide resolved
sagar-salvi-apptware marked this conversation as resolved.
Show resolved Hide resolved
),
)

looker_environment: Literal["prod", "dev"] = Field(
"prod",
description="A looker prod or dev environment. "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,7 @@ class LookerManifest:
# This must be set if the manifest has local_dependency entries.
# See https://cloud.google.com/looker/docs/reference/param-manifest-project-name
project_name: Optional[str]
constants: Optional[List[Dict[str, str]]]

local_dependencies: List[str]
remote_dependencies: List[LookerRemoteDependency]
Expand Down Expand Up @@ -309,6 +310,8 @@ def __init__(self, config: LookMLSourceConfig, ctx: PipelineContext):
"manage_models permission enabled on this API key."
) from err

self.looker_constant: List[Dict[str, str]] = []

def _load_model(self, path: str) -> LookerModel:
logger.debug(f"Loading model from file {path}")

Expand Down Expand Up @@ -506,6 +509,7 @@ def get_manifest_if_present(self, folder: pathlib.Path) -> Optional[LookerManife

manifest = LookerManifest(
project_name=manifest_dict.get("project_name"),
constants=manifest_dict.get("constants", []),
local_dependencies=[
x["project"] for x in manifest_dict.get("local_dependencys", [])
],
Expand Down Expand Up @@ -574,7 +578,7 @@ def get_workunits_internal(self) -> Iterable[MetadataWorkUnit]:
self.base_projects_folder[project] = p_ref

self._recursively_check_manifests(
tmp_dir, BASE_PROJECT_NAME, visited_projects
tmp_dir, BASE_PROJECT_NAME, visited_projects, self.looker_constant
)

yield from self.get_internal_workunits()
Expand All @@ -587,7 +591,11 @@ def get_workunits_internal(self) -> Iterable[MetadataWorkUnit]:
)

def _recursively_check_manifests(
self, tmp_dir: str, project_name: str, project_visited: Set[str]
self,
tmp_dir: str,
project_name: str,
project_visited: Set[str],
looker_constant: List[Dict[str, str]],
) -> None:
if project_name in project_visited:
return
Expand All @@ -604,6 +612,9 @@ def _recursively_check_manifests(
if not manifest:
return

if manifest.constants:
looker_constant.extend(manifest.constants)
sagar-salvi-apptware marked this conversation as resolved.
Show resolved Hide resolved

# 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 (
Expand Down Expand Up @@ -663,20 +674,22 @@ def _recursively_check_manifests(
project_visited.add(project_name)
else:
self._recursively_check_manifests(
tmp_dir, remote_project.name, project_visited
tmp_dir, remote_project.name, project_visited, looker_constant
)

for project in manifest.local_dependencies:
self._recursively_check_manifests(tmp_dir, project, project_visited)
self._recursively_check_manifests(
tmp_dir, project, project_visited, looker_constant
)

def get_internal_workunits(self) -> Iterable[MetadataWorkUnit]: # noqa: C901
assert self.source_config.base_folder

viewfile_loader = LookerViewFileLoader(
self.source_config.project_name,
self.base_projects_folder,
self.reporter,
self.source_config,
self.looker_constant,
)

# Some views can be mentioned by multiple 'include' statements and can be included via different connections.
Expand Down
4 changes: 4 additions & 0 deletions metadata-ingestion/tests/integration/lookml/test_lookml.py
Original file line number Diff line number Diff line change
Expand Up @@ -869,11 +869,15 @@ def test_view_to_view_lineage_and_liquid_template(pytestconfig, tmp_path, mock_t
"dev_database_prefix": "employee",
"dev_schema_prefix": "public",
},
"_dev_database_attributes": {
"dev_database_prefix": "customer",
},
"dw_eff_dt_date": {
"_is_selected": True,
},
"source_region": "ap-south-1",
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need some unit tests focused specifically on LookmlConstantTransformer

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where are the unit tests? we have full integration tests for this, but we should really have a more targeted unit test that creates a LookmlConstantTransformer and tests its behavior in isolation

Copy link
Contributor Author

@sagar-salvi-apptware sagar-salvi-apptware Jan 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi @hsheth2 this is old code new commits will have new test for lookml constant.
0b54bb6

new_recipe["source"]["config"]["lookml_constant"] = {"winner_table": "dev"}

pipeline = Pipeline.create(new_recipe)
pipeline.run()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,6 @@
view: activity_logs {
sql_table_name:
{% if _user_attributes['looker_env'] == 'dev' %}
{{ _user_attributes['dev_database_prefix'] }}analytics.{{ _user_attributes['dev_schema_prefix'] }}staging_app.stg_app__activity_logs
{% elsif _user_attributes['looker_env'] == 'prod' %}
analytics.staging_app.stg_app__activity_logs
{% else %}
analytics.staging_app.stg_app__activity_logs
{% endif %}
;;
{{ _dev_database_attributes['dev_database_prefix'] }}_analytics.{{ _user_attributes['dev_schema_prefix'] }}_staging_app.stg_app__activity_logs;;
sagar-salvi-apptware marked this conversation as resolved.
Show resolved Hide resolved

dimension: generated_message_id {
group_label: "IDs"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ include: "environment_activity_logs.view.lkml"
include: "employee_income_source_as_per_env.view.lkml"
include: "rent_as_employee_income_source.view.lkml"
include: "child_view.view.lkml"
include: "star_award_winner.view.lkml"
include: "star_award_winner_dev.view.lkml"


explore: activity_logs {
}
Expand Down Expand Up @@ -39,4 +42,10 @@ explore: rent_as_employee_income_source {
}

explore: child_view {
}
}

explore: star_award_winner {
}

explore: star_award_winner_dev {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
constant: customer_support_db {
value: "star_award_winner_year"
export: none
}

constant: customer_support_schema {
value: "public"
export: none
}

constant: customer_support_table {
value: "winner"
export: none
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
view: star_award_winner {
sql_table_name: @{customer_support_db}.@{customer_support_schema}.@{customer_support_table};;


dimension: id {
label: "id"
primary_key: yes
type: number
sql: ${TABLE}.id ;;
}

}
Loading
Loading