Skip to content

Commit

Permalink
Do not jinja render packages from dependencies yml (#7910)
Browse files Browse the repository at this point in the history
* Add some comments to methods constructing Project/RuntimeConfig

* Save flag that packages dict came from dependencies.yml

* Test for not rendering packages_dict

* Changie

* Ensure packages_yml_dict and dependencies_yml_dict are dictionaries

* Ensure "packages" passed to render_packages is a dict
  • Loading branch information
gshank committed Jun 21, 2023
1 parent 73a0dc6 commit 9ff2f6e
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 5 deletions.
6 changes: 6 additions & 0 deletions .changes/unreleased/Under the Hood-20230620-180852.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Under the Hood
body: Don't jinja render packages from dependencies.yml
time: 2023-06-20T18:08:52.848093-04:00
custom:
Author: gshank
Issue: "7905"
13 changes: 10 additions & 3 deletions core/dbt/config/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,9 @@ def package_and_project_data_from_root(project_root):
packages_yml_dict = {}
dependencies_yml_dict = {}
if path_exists(package_filepath):
packages_yml_dict = _load_yaml(package_filepath)
packages_yml_dict = _load_yaml(package_filepath) or {}
if path_exists(dependencies_filepath):
dependencies_yml_dict = _load_yaml(dependencies_filepath)
dependencies_yml_dict = _load_yaml(dependencies_filepath) or {}

if "packages" in packages_yml_dict and "packages" in dependencies_yml_dict:
msg = "The 'packages' key cannot be specified in both packages.yml and dependencies.yml"
Expand All @@ -116,6 +116,7 @@ def package_and_project_data_from_root(project_root):
dependent_projects_dict = {}
if "packages" in dependencies_yml_dict:
packages_dict["packages"] = dependencies_yml_dict["packages"]
packages_dict["packages_from_dependencies"] = True
else: # don't check for "packages" here so we capture invalid keys in packages.yml
packages_dict = packages_yml_dict
if "projects" in dependencies_yml_dict:
Expand Down Expand Up @@ -283,6 +284,7 @@ class RenderComponents:

@dataclass
class PartialProject(RenderComponents):
# This class includes the project_dict, packages_dict, selectors_dict, etc from RenderComponents
profile_name: Optional[str] = field(
metadata=dict(description="The unrendered profile name in the project, if set")
)
Expand Down Expand Up @@ -324,7 +326,7 @@ def get_rendered(
selectors_dict=rendered_selectors,
)

# Called by 'collect_parts' in RuntimeConfig
# Called by Project.from_project_root (not PartialProject.from_project_root!)
def render(self, renderer: DbtProjectYamlRenderer) -> "Project":
try:
rendered = self.get_rendered(renderer)
Expand Down Expand Up @@ -538,6 +540,7 @@ def from_dicts(
project_name = project_dict.get("name")
profile_name = project_dict.get("profile")

# Create a PartialProject
return cls(
profile_name=profile_name,
project_name=project_name,
Expand All @@ -555,6 +558,7 @@ def from_project_root(
) -> "PartialProject":
project_root = os.path.normpath(project_root)
project_dict = load_raw_project(project_root)
# Read packages.yml and dependencies.yml and pass dictionaries to "from_dicts" method
packages_dict, dependent_projects_dict = package_and_project_data_from_root(project_root)
selectors_dict = selector_data_from_root(project_root)
return cls.from_dicts(
Expand Down Expand Up @@ -715,6 +719,9 @@ def partial_load(cls, project_root: str, *, verify_version: bool = False) -> Par
verify_version=verify_version,
)

# Called by:
# RtConfig.load_dependencies => RtConfig.load_projects => RtConfig.new_project => Project.from_project_root
# RtConfig.from_args => RtConfig.collect_parts => load_project => Project.from_project_root
@classmethod
def from_project_root(
cls,
Expand Down
8 changes: 7 additions & 1 deletion core/dbt/config/renderer.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,14 @@ def render_project(

def render_packages(self, packages: Dict[str, Any]):
"""Render the given packages dict"""
packages = packages or {} # Sometimes this is none in tests
package_renderer = self.get_package_renderer()
return package_renderer.render_data(packages)
if "packages_from_dependencies" in packages:
# We don't want to render the "packages" dictionary that came from dependencies.yml
packages.pop("packages_from_dependencies")
return packages
else:
return package_renderer.render_data(packages)

def render_dependent_projects(self, dependent_projects: Dict[str, Any]):
"""This is a no-op to maintain regularity in the interfaces. We don't render dependencies.yml."""
Expand Down
4 changes: 3 additions & 1 deletion core/dbt/config/runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
from .renderer import DbtProjectYamlRenderer, ProfileRenderer


# Called by RuntimeConfig.collect_parts class method
def load_project(
project_root: str,
version_check: bool,
Expand Down Expand Up @@ -237,6 +238,7 @@ def validate(self):
except ValidationError as e:
raise ConfigContractBrokenError(e) from e

# Called by RuntimeConfig.from_args
@classmethod
def collect_parts(cls: Type["RuntimeConfig"], args: Any) -> Tuple[Project, Profile]:
# profile_name from the project
Expand All @@ -251,7 +253,7 @@ def collect_parts(cls: Type["RuntimeConfig"], args: Any) -> Tuple[Project, Profi
project = load_project(project_root, bool(flags.VERSION_CHECK), profile, cli_vars)
return project, profile

# Called in main.py, lib.py, task/base.py
# Called in task/base.py, in BaseTask.from_args
@classmethod
def from_args(cls, args: Any) -> "RuntimeConfig":
"""Given arguments, read in dbt_project.yml from the current directory,
Expand Down
10 changes: 10 additions & 0 deletions tests/functional/dependencies/test_simple_dependency.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,16 @@ def test_dependency_with_dependencies_file(self, run_deps, project):
assert len(results) == 4


class TestSimpleDependencyWithEmptyPackagesFile(SimpleDependencyBase):
@pytest.fixture(scope="class")
def packages(self):
return " "

def test_dependency_with_empty_packages_file(self, run_deps, project):
# Tests that an empty packages file doesn't fail with a Python error
run_dbt(["deps"])


class TestSimpleDependencyNoProfile(SimpleDependencyBase):
"""dbt deps and clean commands should not require a profile."""

Expand Down
16 changes: 16 additions & 0 deletions tests/unit/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -918,6 +918,22 @@ def test_custom_query_comment_append(self):
self.assertEqual(project.query_comment.comment, "run by user test")
self.assertEqual(project.query_comment.append, True)

def test_packages_from_dependencies(self):
packages = {
"packages_from_dependencies": True,
"packages": [
{
"git": "{{ env_var('some_package') }}",
"warn-unpinned": True,
}
],
}

project = project_from_config_rendered(self.default_project_data, packages)
git_package = project.packages.packages[0]
# packages did not render because of "packages_from_dependencies" key
assert git_package.git == "{{ env_var('some_package') }}"


class TestProjectFile(BaseFileTest):
def setUp(self):
Expand Down

0 comments on commit 9ff2f6e

Please sign in to comment.