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
Original file line number Diff line number Diff line change
Expand Up @@ -198,9 +198,9 @@
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 +287,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 +335,56 @@
return self._apply_regx(value)


class LookmlParameterTransformer(LookMLViewTransformer):
"""
Replace LookML parameters (@{param} or ${var}) from the configuration.
"""

PARAMETER_PATTERN = r"@{(\w+)}" # Matches @{param}
LIQUID_VARIABLE_PATTERN = r"\${(\w+)}" # Matches ${var}
sgomezvillamor marked this conversation as resolved.
Show resolved Hide resolved

def resolve_lookml_parameter(self, text: str) -> str:
"""
Resolves LookML parameters (@{param}) and liquid variables (${var}).
Logs warnings for misplaced or missing variables.
"""

def replace_parameters(match):
key = match.group(1)
# 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 356 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#L356

Added line #L356 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 359 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#L359

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

# Resolve parameter
if key in self.source_config.lookml_parameter:
return str(self.source_config.lookml_parameter.get(key))

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

Check warning on line 366 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#L365-L366

Added lines #L365 - L366 were not covered by tests

def replace_liquid_variables(match):
key = match.group(1)
# Resolve liquid variable
if key in self.source_config.liquid_variable:
return str(self.source_config.liquid_variable.get(key))

logger.warning(f"Liquid variable '${{{key}}}' not found in configuration.")
return ""

Check warning on line 375 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#L374-L375

Added lines #L374 - L375 were not covered by tests

# Resolve @{param} (parameters)
text = re.sub(self.PARAMETER_PATTERN, replace_parameters, text)

# Resolve ${var} (liquid variables)
text = re.sub(self.LIQUID_VARIABLE_PATTERN, replace_liquid_variables, text)
sagar-salvi-apptware marked this conversation as resolved.
Show resolved Hide resolved
return 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 @@ -401,6 +451,9 @@
LiquidVariableTransformer(
source_config=source_config
), # Now resolve liquid variables
LookmlParameterTransformer(
source_config=source_config
), # Remove @{param}/${var} with its corresponding value
sagar-salvi-apptware marked this conversation as resolved.
Show resolved Hide resolved
DropDerivedViewPatternTransformer(
source_config=source_config
), # Remove any ${} symbol
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,15 @@ class LookMLSourceConfig(
"view.sql_table_name. Defaults to an empty dictionary.",
)

lookml_parameter: Dict[Any, Any] = Field(
sagar-salvi-apptware marked this conversation as resolved.
Show resolved Hide resolved
{},
description=(
"A dictionary containing LookML parameters (`@{param_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
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 @@ -873,6 +873,10 @@ def test_view_to_view_lineage_and_liquid_template(pytestconfig, tmp_path, mock_t
"_is_selected": True,
},
"source_region": "ap-south-1",
"db": "test-db",
}
new_recipe["source"]["config"]["lookml_parameter"] = {
"star_award_winner_year": "public.winner_2025"
}
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


pipeline = Pipeline.create(new_recipe)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ 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"

explore: activity_logs {
}
Expand Down Expand Up @@ -39,4 +40,7 @@ explore: rent_as_employee_income_source {
}

explore: child_view {
}

explore: star_award_winner {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
view: star_award_winner {
sql_table_name: ${db}.@{star_award_winner_year};;


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

parameter: star_award_winner_year {
type: string
allowed_value: {
label: "Star Award Winner Of 2025"
value: "public.winner_2025"
}
allowed_value: {
label: "Star Award Winner Of 2024"
value: "public.winner_2024"
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -2827,6 +2827,192 @@
"lastRunId": "no-run-id-provided"
}
},
{
"entityType": "dataset",
"entityUrn": "urn:li:dataset:(urn:li:dataPlatform:looker,lkml_samples.view.star_award_winner,PROD)",
"changeType": "UPSERT",
"aspectName": "subTypes",
"aspect": {
"json": {
"typeNames": [
"View"
]
}
},
"systemMetadata": {
"lastObserved": 1586847600000,
"runId": "lookml-test",
"lastRunId": "no-run-id-provided"
}
},
{
"entityType": "dataset",
"entityUrn": "urn:li:dataset:(urn:li:dataPlatform:looker,lkml_samples.view.star_award_winner,PROD)",
"changeType": "UPSERT",
"aspectName": "viewProperties",
"aspect": {
"json": {
"materialized": false,
"viewLogic": "view: star_award_winner {\n sql_table_name: ${db}.@{star_award_winner_year};;\n\n\n dimension: id {\n label: \"id\"\n primary_key: yes\n type: number\n sql: ${TABLE}.id ;;\n }\n\n parameter: star_award_winner_year {\n type: string\n allowed_value: {\n label: \"Star Award Winner Of 2025\"\n value: \"public.winner_2025\"\n }\n allowed_value: {\n label: \"Star Award Winner Of 2024\"\n value: \"public.winner_2024\"\n }\n }\n\n}",
"viewLanguage": "lookml"
}
},
"systemMetadata": {
"lastObserved": 1586847600000,
"runId": "lookml-test",
"lastRunId": "no-run-id-provided"
}
},
{
"entityType": "dataset",
"entityUrn": "urn:li:dataset:(urn:li:dataPlatform:looker,lkml_samples.view.star_award_winner,PROD)",
"changeType": "UPSERT",
"aspectName": "container",
"aspect": {
"json": {
"container": "urn:li:container:78f22c19304954b15e8adb1d9809975e"
}
},
"systemMetadata": {
"lastObserved": 1586847600000,
"runId": "lookml-test",
"lastRunId": "no-run-id-provided"
}
},
{
"proposedSnapshot": {
"com.linkedin.pegasus2avro.metadata.snapshot.DatasetSnapshot": {
"urn": "urn:li:dataset:(urn:li:dataPlatform:looker,lkml_samples.view.star_award_winner,PROD)",
"aspects": [
{
"com.linkedin.pegasus2avro.common.BrowsePaths": {
"paths": [
"/Develop/lkml_samples/"
]
}
},
{
"com.linkedin.pegasus2avro.common.Status": {
"removed": false
}
},
{
"com.linkedin.pegasus2avro.dataset.UpstreamLineage": {
"upstreams": [
{
"auditStamp": {
"time": 1586847600000,
"actor": "urn:li:corpuser:datahub"
},
"dataset": "urn:li:dataset:(urn:li:dataPlatform:postgres,test-db.public.winner_2025,PROD)",
"type": "VIEW"
}
],
"fineGrainedLineages": [
{
"upstreamType": "FIELD_SET",
"upstreams": [
"urn:li:schemaField:(urn:li:dataset:(urn:li:dataPlatform:postgres,test-db.public.winner_2025,PROD),id)"
],
"downstreamType": "FIELD",
"downstreams": [
"urn:li:schemaField:(urn:li:dataset:(urn:li:dataPlatform:looker,lkml_samples.view.star_award_winner,PROD),id)"
],
"confidenceScore": 1.0
}
]
}
},
{
"com.linkedin.pegasus2avro.schema.SchemaMetadata": {
"schemaName": "star_award_winner",
"platform": "urn:li:dataPlatform:looker",
"version": 0,
"created": {
"time": 0,
"actor": "urn:li:corpuser:unknown"
},
"lastModified": {
"time": 0,
"actor": "urn:li:corpuser:unknown"
},
"hash": "",
"platformSchema": {
"com.linkedin.pegasus2avro.schema.OtherSchema": {
"rawSchema": ""
}
},
"fields": [
{
"fieldPath": "id",
"nullable": false,
"description": "",
"label": "id",
"type": {
"type": {
"com.linkedin.pegasus2avro.schema.NumberType": {}
}
},
"nativeDataType": "number",
"recursive": false,
"globalTags": {
"tags": [
{
"tag": "urn:li:tag:Dimension"
}
]
},
"isPartOfKey": true
}
],
"primaryKeys": [
"id"
]
}
},
{
"com.linkedin.pegasus2avro.dataset.DatasetProperties": {
"customProperties": {
"looker.file.path": "star_award_winner.view.lkml",
"looker.model": "data"
},
"name": "star_award_winner",
"tags": []
}
}
]
}
},
"systemMetadata": {
"lastObserved": 1586847600000,
"runId": "lookml-test",
"lastRunId": "no-run-id-provided"
}
},
{
"entityType": "dataset",
"entityUrn": "urn:li:dataset:(urn:li:dataPlatform:looker,lkml_samples.view.star_award_winner,PROD)",
"changeType": "UPSERT",
"aspectName": "browsePathsV2",
"aspect": {
"json": {
"path": [
{
"id": "Develop"
},
{
"id": "urn:li:container:78f22c19304954b15e8adb1d9809975e",
"urn": "urn:li:container:78f22c19304954b15e8adb1d9809975e"
}
]
}
},
"systemMetadata": {
"lastObserved": 1586847600000,
"runId": "lookml-test",
"lastRunId": "no-run-id-provided"
}
},
{
"entityType": "tag",
"entityUrn": "urn:li:tag:Dimension",
Expand Down
Loading