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

[Backport 1.5.latest] Fix uncaught exception for group updates #8813

Merged
merged 1 commit into from
Oct 10, 2023
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-20231010-125948.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Fixes
body: Group updates on unmodified nodes are handled gracefully for state:modified
time: 2023-10-10T12:59:48.390113-05:00
custom:
Author: emmyoop
Issue: "8371"
9 changes: 8 additions & 1 deletion core/dbt/contracts/graph/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -853,7 +853,14 @@ def build_group_map(self):
group_map = {group.name: [] for group in self.groups.values()}
for node in groupable_nodes:
if node.group is not None:
group_map[node.group].append(node.unique_id)
# group updates are not included with state:modified and
# by ignoring the groups that aren't in the group map we
# can avoid hitting errors for groups that are not getting
# updated. This is a hack but any groups that are not
# valid will be caught in
# parser.manifest.ManifestLoader.check_valid_group_config_node
if node.group in group_map:
group_map[node.group].append(node.unique_id)
self.group_map = group_map

def writable_manifest(self):
Expand Down
63 changes: 63 additions & 0 deletions tests/functional/defer_state/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -247,3 +247,66 @@

{% endsnapshot %}
"""

model_1_sql = """
select * from {{ ref('seed') }}
"""

modified_model_1_sql = """
select * from {{ ref('seed') }}
order by 1
"""

model_2_sql = """
select id from {{ ref('model_1') }}
"""

modified_model_2_sql = """
select * from {{ ref('model_1') }}
order by 1
"""


group_schema_yml = """
groups:
- name: finance
owner:
email: [email protected]

models:
- name: model_1
config:
group: finance
- name: model_2
config:
group: finance
"""


group_modified_schema_yml = """
groups:
- name: accounting
owner:
email: [email protected]
models:
- name: model_1
config:
group: accounting
- name: model_2
config:
group: accounting
"""

group_modified_fail_schema_yml = """
groups:
- name: finance
owner:
email: [email protected]
models:
- name: model_1
config:
group: accounting
- name: model_2
config:
group: finance
"""
121 changes: 121 additions & 0 deletions tests/functional/defer_state/test_group_updates.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
import os

import pytest

from dbt.tests.util import run_dbt, write_file, copy_file
from dbt.exceptions import ParsingError


from tests.functional.defer_state.fixtures import (
seed_csv,
model_1_sql,
modified_model_1_sql,
model_2_sql,
modified_model_2_sql,
group_schema_yml,
group_modified_schema_yml,
group_modified_fail_schema_yml,
)


class GroupSetup:
@pytest.fixture(scope="class")
def models(self):
return {
"model_1.sql": model_1_sql,
"model_2.sql": model_2_sql,
"schema.yml": group_schema_yml,
}

@pytest.fixture(scope="class")
def seeds(self):
return {
"seed.csv": seed_csv,
}

def group_setup(self):
# save initial state
run_dbt(["seed"])
results = run_dbt(["compile"])

# add sanity checks for first result
assert len(results) == 3
seed_result = results[0].node
assert seed_result.unique_id == "seed.test.seed"
model_1_result = results[1].node
assert model_1_result.unique_id == "model.test.model_1"
assert model_1_result.group == "finance"
model_2_result = results[2].node
assert model_2_result.unique_id == "model.test.model_2"
assert model_2_result.group == "finance"


class TestFullyModifiedGroups(GroupSetup):
def test_changed_groups(self, project):
self.group_setup()

# copy manifest.json to "state" directory
os.makedirs("state")
target_path = os.path.join(project.project_root, "target")
copy_file(target_path, "manifest.json", project.project_root, ["state", "manifest.json"])

# update group name, modify model so it gets picked up
write_file(modified_model_1_sql, "models", "model_1.sql")
write_file(modified_model_2_sql, "models", "model_2.sql")
write_file(group_modified_schema_yml, "models", "schema.yml")

# this test is flaky if you don't clean first before the build
run_dbt(["clean"])
# only thing in results should be model_1
results = run_dbt(["build", "-s", "state:modified", "--defer", "--state", "./state"])

assert len(results) == 2
model_1_result = results[0].node
assert model_1_result.unique_id == "model.test.model_1"
assert model_1_result.group == "accounting" # new group name!
model_2_result = results[1].node
assert model_2_result.unique_id == "model.test.model_2"
assert model_2_result.group == "accounting" # new group name!


class TestPartiallyModifiedGroups(GroupSetup):
def test_changed_groups(self, project):
self.group_setup()

# copy manifest.json to "state" directory
os.makedirs("state")
target_path = os.path.join(project.project_root, "target")
copy_file(target_path, "manifest.json", project.project_root, ["state", "manifest.json"])

# update group name, modify model so it gets picked up
write_file(modified_model_1_sql, "models", "model_1.sql")
write_file(group_modified_schema_yml, "models", "schema.yml")

# this test is flaky if you don't clean first before the build
run_dbt(["clean"])
# only thing in results should be model_1
results = run_dbt(["build", "-s", "state:modified", "--defer", "--state", "./state"])

assert len(results) == 1
model_1_result = results[0].node
assert model_1_result.unique_id == "model.test.model_1"
assert model_1_result.group == "accounting" # new group name!


class TestBadGroups(GroupSetup):
def test_changed_groups(self, project):
self.group_setup()

# copy manifest.json to "state" directory
os.makedirs("state")
target_path = os.path.join(project.project_root, "target")
copy_file(target_path, "manifest.json", project.project_root, ["state", "manifest.json"])

# update group with invalid name, modify model so it gets picked up
write_file(modified_model_1_sql, "models", "model_1.sql")
write_file(group_modified_fail_schema_yml, "models", "schema.yml")

# this test is flaky if you don't clean first before the build
run_dbt(["clean"])
with pytest.raises(ParsingError, match="Invalid group 'accounting'"):
run_dbt(["build", "-s", "state:modified", "--defer", "--state", "./state"])
Loading