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

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
fa4f5f2
lookml parameter support
sid-acryl Dec 19, 2024
865b5ff
fix: pr comment
sagar-salvi-apptware Jan 8, 2025
36f746f
test: updated golden files
sagar-salvi-apptware Jan 8, 2025
4a9ec88
fix: minor change
sagar-salvi-apptware Jan 8, 2025
750cf33
fix: pr comments
sagar-salvi-apptware Jan 8, 2025
4e7bc94
fix: pr comment
sagar-salvi-apptware Jan 10, 2025
ae605d9
fix: refactor changes to lookml constant
sagar-salvi-apptware Jan 15, 2025
a7cc9d5
test: updated test for lookml constant
sagar-salvi-apptware Jan 15, 2025
46316ca
fix: fixed lint issue
sagar-salvi-apptware Jan 15, 2025
2c16fc6
fix: pr comments
sagar-salvi-apptware Jan 16, 2025
0b54bb6
test: updated test for lookml constant
sagar-salvi-apptware Jan 16, 2025
f08b827
fix: pr comment
sagar-salvi-apptware Jan 17, 2025
f1daf12
fix: rename liquid_variable -> liquid_variables
sagar-salvi-apptware Jan 18, 2025
0b85dbd
fix: pr comments
sagar-salvi-apptware Jan 21, 2025
22b5af3
test: add unit tests for lookml constant trasnformer
sagar-salvi-apptware Jan 21, 2025
d68fd5d
fix: minor change
sagar-salvi-apptware Jan 21, 2025
896bf90
fix: minor comments
sagar-salvi-apptware Jan 21, 2025
69fed36
fix: pr comment
sagar-salvi-apptware Jan 22, 2025
8adeb4c
fix: minor pr comments
sagar-salvi-apptware Jan 23, 2025
baf4278
Update metadata-ingestion/docs/sources/looker/lookml_post.md
sagar-salvi-apptware Jan 23, 2025
41422a7
Update metadata-ingestion/docs/sources/looker/lookml_post.md
sagar-salvi-apptware Jan 23, 2025
f4fb5d7
Merge branch 'master' into fix/ING-784-lookml-parameter
mayurinehate Jan 23, 2025
ee02fcd
fix: added report info
sagar-salvi-apptware Jan 24, 2025
6634d1e
test: fixed ut
sagar-salvi-apptware Jan 24, 2025
aa89283
fix: added minor changes
sagar-salvi-apptware Jan 24, 2025
5726a26
fix: Constant Resolution
sagar-salvi-apptware Jan 24, 2025
00d1392
test: Constant Resolution
sagar-salvi-apptware Jan 24, 2025
3d1443a
Update metadata-ingestion/src/datahub/ingestion/source/looker/looker_…
sagar-salvi-apptware Jan 27, 2025
0a4cd06
Update metadata-ingestion/src/datahub/ingestion/source/looker/looker_…
sagar-salvi-apptware Jan 27, 2025
250b876
Update metadata-ingestion/tests/integration/lookml/test_lookml.py
sagar-salvi-apptware Jan 27, 2025
c5d8b2d
fix: minor pr comments
sagar-salvi-apptware Jan 27, 2025
d592a52
Merge branch 'master' into fix/ING-784-lookml-parameter
mayurinehate Jan 27, 2025
31a1152
Merge branch 'master' into fix/ING-784-lookml-parameter
mayurinehate Jan 28, 2025
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
Prev Previous commit
Next Next commit
fix: minor pr comments
  • Loading branch information
sagar-salvi-apptware committed Jan 23, 2025
commit 8adeb4c319616d9f9cef178f0ee3286314291a38
51 changes: 37 additions & 14 deletions metadata-ingestion/docs/sources/looker/lookml_post.md
Original file line number Diff line number Diff line change
@@ -1,27 +1,50 @@
#### Configuration Notes
### 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_variables` config as shown below:
1. Handling Liquid Templates

If a view contains a liquid template, for example:

```
sql_table_name: {{ user_attributes['db'] }}.kafka_streaming.events
```

where `db=ANALYTICS_PROD`, you need to specify the values of those variables in the liquid_variables configuration as shown below:

```yml
liquid_variables:
user_attributes:
db: ANALYTICS_PROD
```

2. If a view contains a lookml constant (e.g., `sql_table_name: @{db}.kafka_streaming.events;`), its value is resolved in the following order:
2. Resolving LookML Constants

If a view contains a LookML constant, for example:

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

its value is resolved in the following order:

- First, checks the `lookml_constants` configuration.

```yml
lookml_constants:
db: ANALYTICS_PROD
```

- If not found, falls back to `manifest.lkml`:

```yml
manifest.lkml
constant: db {
value: "ANALYTICS_PROD"
}
```

- **First, checks the `manifest.lkml` file** for the constant definition.
```manifest.lkml
constant: db {
value: "ANALYTICS_PROD"
}
```
- **If not found, falls back to `config`**
**Additional Notes**

```yml
lookml_constants:
db: ANALYTICS_PROD
```
Although liquid variables and LookML constants can be used anywhere in LookML code, their values are currently resolved only for LookML views. This behavior is sufficient since DataHub LookML ingestion processes only views and their upstream dependencies.

### Multi-Project LookML (Advanced)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -392,10 +392,10 @@
if key in self.source_config.liquid_variables:
self.reporter.report_warning(
title="Misplaced lookml constant",
message="Misplaced lookml constant, Use 'lookml_constants' instead of 'liquid_variables'.",
message="Use 'lookml_constants' instead of 'liquid_variables'.",
context=f"Key {key}",
)
return f"@{{{key}}}"

Check warning on line 398 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#L398

Added line #L398 was not covered by tests

logger.warning(f"Constant '@{{{key}}}' not found in configuration.")
return "NULL"
Expand Down
10 changes: 5 additions & 5 deletions metadata-ingestion/tests/integration/lookml/test_lookml.py
Original file line number Diff line number Diff line change
Expand Up @@ -1056,8 +1056,8 @@ def test_special_liquid_variables():
),
# Case 10: Misplaced lookml constant
(
{"sql_table_name": "@{liquid1}.@{constant1}"},
{"datahub_transformed_sql_table_name": "@{liquid1}.value1"},
{"sql_table_name": "@{constant1}.@{constant2}.@{constant4}"},
{"datahub_transformed_sql_table_name": "value1.value2.@{constant4}"},
True,
),
],
Expand All @@ -1074,7 +1074,7 @@ def test_lookml_constant_transformer(view, expected_result, warning_expected):
"constant2": "value2",
}
config.liquid_variables = {
"liquid1": "liquid_value1",
"constant4": "liquid_value1",
}

transformer = LookmlConstantTransformer(
Expand All @@ -1091,8 +1091,8 @@ def test_lookml_constant_transformer(view, expected_result, warning_expected):
if warning_expected:
report.report_warning.assert_called_once_with(
title="Misplaced lookml constant",
message="Misplaced lookml constant, Use 'lookml_constants' instead of 'liquid_variables'.",
context="Key liquid1",
message="Use 'lookml_constants' instead of 'liquid_variables'.",
context="Key constant4",
)


Expand Down
Loading