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

Fix #9534: Add NodeRelation to SavedQuery Export #9794

Merged
merged 6 commits into from
Apr 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions .changes/unreleased/Fixes-20240410-181741.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Fixes
body: Add NodeRelation to SavedQuery Export
time: 2024-04-10T18:17:41.42533+01:00
custom:
Author: aranke
Issue: "9534"
1 change: 1 addition & 0 deletions core/dbt/artifacts/resources/v1/saved_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ class ExportConfig(dbtClassMixin):
export_as: ExportDestinationType
schema_name: Optional[str] = None
alias: Optional[str] = None
database: Optional[str] = None


@dataclass
Expand Down
6 changes: 3 additions & 3 deletions core/dbt/parser/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ def __init__(self, config: RuntimeConfig, manifest: Manifest, component: str) ->
self.component = component

def __call__(self, parsed_node: Any, override: Optional[str]) -> None:
if parsed_node.package_name in self.package_updaters:
if getattr(parsed_node, "package_name", None) in self.package_updaters:
new_value = self.package_updaters[parsed_node.package_name](override, parsed_node)
else:
new_value = self.default_updater(override, parsed_node)
Expand Down Expand Up @@ -293,7 +293,7 @@ def update_parsed_node_relation_names(
self._update_node_alias(parsed_node, config_dict.get("alias"))

# Snapshot nodes use special "target_database" and "target_schema" fields for some reason
if parsed_node.resource_type == NodeType.Snapshot:
if getattr(parsed_node, "resource_type", None) == NodeType.Snapshot:
if "target_database" in config_dict and config_dict["target_database"]:
parsed_node.database = config_dict["target_database"]
if "target_schema" in config_dict and config_dict["target_schema"]:
Expand Down Expand Up @@ -452,7 +452,7 @@ def _update_node_relation_name(self, node: ManifestNode):
# and TestNodes that store_failures.
# TestNodes do not get a relation_name without store failures
# because no schema is created.
if node.is_relational and not node.is_ephemeral_model:
if getattr(node, "is_relational", None) and not getattr(node, "is_ephemeral_model", None):
adapter = get_adapter(self.root_project)
relation_cls = adapter.Relation
node.relation_name = str(relation_cls.create_from(self.root_project, node))
Expand Down
16 changes: 16 additions & 0 deletions core/dbt/parser/schema_yaml_readers.py
Original file line number Diff line number Diff line change
Expand Up @@ -764,6 +764,22 @@
group=config.group,
)

for export in parsed.exports:
self.schema_parser.update_parsed_node_relation_names(export, export.config.to_dict()) # type: ignore

Check warning on line 768 in core/dbt/parser/schema_yaml_readers.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/parser/schema_yaml_readers.py#L767-L768

Added lines #L767 - L768 were not covered by tests

if not export.config.schema_name:
export.config.schema_name = getattr(export, "schema", None)
delattr(export, "schema")

Check warning on line 772 in core/dbt/parser/schema_yaml_readers.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/parser/schema_yaml_readers.py#L770-L772

Added lines #L770 - L772 were not covered by tests

export.config.database = getattr(export, "database", None) or export.config.database
delattr(export, "database")

Check warning on line 775 in core/dbt/parser/schema_yaml_readers.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/parser/schema_yaml_readers.py#L774-L775

Added lines #L774 - L775 were not covered by tests

if not export.config.alias:
export.config.alias = getattr(export, "alias", None)
delattr(export, "alias")

Check warning on line 779 in core/dbt/parser/schema_yaml_readers.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/parser/schema_yaml_readers.py#L777-L779

Added lines #L777 - L779 were not covered by tests

delattr(export, "relation_name")

Check warning on line 781 in core/dbt/parser/schema_yaml_readers.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/parser/schema_yaml_readers.py#L781

Added line #L781 was not covered by tests

# Only add thes saved query if it's enabled, otherwise we track it with other diabled nodes
if parsed.config.enabled:
self.manifest.add_saved_query(self.yaml.file, parsed)
Expand Down
23 changes: 23 additions & 0 deletions tests/functional/saved_queries/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,29 @@
schema: my_export_schema_name
"""

saved_queries_with_defaults_yml = """
version: 2

saved_queries:
- name: test_saved_query
description: "{{ doc('saved_query_description') }}"
label: Test Saved Query
query_params:
metrics:
- simple_metric
group_by:
- "Dimension('user__ds')"
where:
- "{{ Dimension('user__ds', 'DAY') }} <= now()"
- "{{ Dimension('user__ds', 'DAY') }} >= '2023-01-01'"
- "{{ Metric('txn_revenue', ['id']) }} > 1"
exports:
- name: my_export
config:
alias: my_export_alias
export_as: table
"""

saved_queries_with_diff_filters_yml = """
version: 2

Expand Down
30 changes: 30 additions & 0 deletions tests/functional/saved_queries/test_configs.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
saved_query_with_extra_config_attributes_yml,
saved_query_with_export_configs_defined_at_saved_query_level_yml,
saved_query_without_export_configs_defined_yml,
saved_queries_with_defaults_yml,
)
from tests.functional.semantic_models.fixtures import (
fct_revenue_sql,
Expand Down Expand Up @@ -121,6 +122,33 @@ def test_extra_config_properties_dont_break_parsing(self, project):
assert saved_query.exports[0].config.__dict__.get("my_random_config") is None


class TestExportConfigsWithDefaultProperties(BaseConfigProject):
@pytest.fixture(scope="class")
def models(self):
return {
"saved_queries.yml": saved_queries_with_defaults_yml,
"schema.yml": schema_yml,
"fct_revenue.sql": fct_revenue_sql,
"metricflow_time_spine.sql": metricflow_time_spine_sql,
"docs.md": saved_query_description,
}

def test_default_properties(self, project):
runner = dbtTestRunner()

# parse with default fixture project config
result = runner.invoke(["parse"])
assert result.success
assert isinstance(result.result, Manifest)
assert len(result.result.saved_queries) == 1
saved_query = result.result.saved_queries["saved_query.test.test_saved_query"]
assert len(saved_query.exports) == 1
export = saved_query.exports[0]
assert export.config.alias == "my_export_alias"
assert export.config.schema_name == project.test_schema
assert export.config.database == project.database


class TestInheritingExportConfigFromSavedQueryConfig(BaseConfigProject):
@pytest.fixture(scope="class")
def models(self):
Expand Down Expand Up @@ -152,6 +180,7 @@ def test_export_config_inherits_from_saved_query(self, project):
assert export1.config.export_as != saved_query.config.export_as
assert export1.config.schema_name == "my_custom_export_schema"
assert export1.config.schema_name != saved_query.config.schema
assert export1.config.database == project.database

# assert Export `my_export` has its configs defined from the saved_query because they should take priority
export2 = next(
Expand All @@ -162,6 +191,7 @@ def test_export_config_inherits_from_saved_query(self, project):
assert export2.config.export_as == saved_query.config.export_as
assert export2.config.schema_name == "my_default_export_schema"
assert export2.config.schema_name == saved_query.config.schema
assert export2.config.database == project.database


class TestInheritingExportConfigsFromProject(BaseConfigProject):
Expand Down