repo-mirrors/dbt-core
1
0
Fork 1
mirror of https://github.com/dbt-labs/dbt-core synced 2025-12-17 19:31:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix --add-package when warn-unpinned: false present (#12233)

Browse Source 
* add tests

fix tests

* deal with bool

* fix bug

* changelog
This commit is contained in:
Emily Rockman
2025-12-01 13:03:37 -05:00
committed by GitHub
parent 3de0160b00
commit c559848044
4 changed files with 329 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
.changes/unreleased/Fixes-20251128-161937.yaml Normal file
View File

@@ -0,0 +1,7 @@
kind: Fixes
body: ':bug: :snowman: Fix `dbt deps --add-package` crash when packages.yml contains `warn-unpinned:
false`'
time: 2025-11-28T16:19:37.608722-05:00
custom:
Author: emmyoop
Issue: "9104"

41
core/dbt/task/deps.py
View File

@@ -119,23 +119,46 @@ class DepsTask(BaseTask):
)
def check_for_duplicate_packages(self, packages_yml):
"""Loop through contents of `packages.yml` to ensure no duplicate package names + versions.
"""Loop through contents of `packages.yml` to remove entries that match the package being added.
This duplicate check will take into consideration exact match of a package name, as well as
a check to see if a package name exists within a name (i.e. a package name inside a git URL).
This method is called only during `dbt deps --add-package` to check if the package
being added already exists in packages.yml. It uses substring matching to identify
duplicates, checking if the package name appears within package identifiers (such as
within git URLs, hub package names, or local paths).
Args:
packages_yml (dict): In-memory read of `packages.yml` contents
Returns:
dict: Updated or untouched packages_yml contents
dict: Updated packages_yml contents with matching packages removed
"""
for i, pkg_entry in enumerate(packages_yml["packages"]):
for val in pkg_entry.values():
if self.args.add_package["name"] in val:
del packages_yml["packages"][i]
# Iterate backwards to safely delete items without index shifting issues
for i in range(len(packages_yml["packages"]) - 1, -1, -1):
pkg_entry = packages_yml["packages"][i]
fire_event(DepsFoundDuplicatePackage(removed_package=pkg_entry))
# Get the package identifier key (package type determines which key exists)
# This avoids iterating over non-string values like warn-unpinned: false
package_identifier = (
pkg_entry.get("package") # hub/registry package
or pkg_entry.get("git") # git package
or pkg_entry.get("local") # local package
or pkg_entry.get("tarball") # tarball package
or pkg_entry.get("private") # private package
)
# Check if package name appears in the identifier using substring match
if package_identifier and self.args.add_package["name"] in package_identifier:
del packages_yml["packages"][i]
# Filter out non-string values (like warn-unpinned boolean) before logging
# Note: Check for bool first since bool is a subclass of int in Python
loggable_package = {
k: v
for k, v in pkg_entry.items()
if not isinstance(v, bool)
and isinstance(v, (str, int, float))
and k != "unrendered"
}
fire_event(DepsFoundDuplicatePackage(removed_package=loggable_package))
return packages_yml

49
tests/functional/dependencies/test_add_package_edge_cases.py Normal file
View File

@@ -0,0 +1,49 @@
import os
import shutil
import pytest
from dbt.tests.util import run_dbt
class TestAddPackageWithWarnUnpinnedInYaml:
"""Functional test: Adding packages works even with warn-unpinned in packages.yml.
This is a regression test for issue #9104. The bug occurred when packages.yml
contained warn-unpinned: false and dbt deps --add-package was run. The code
would fail with "TypeError: argument of type 'bool' is not iterable".
"""
@pytest.fixture(scope="class")
def packages(self):
# Start with a git package that has warn-unpinned (matching the bug report)
return {
"packages": [
{
"git": "https://github.com/fivetran/dbt_amplitude",
"warn-unpinned": False, # This is the config that caused the bug
},
]
}
@pytest.fixture
def clean_start(self, project):
if os.path.exists("dbt_packages"):
shutil.rmtree("dbt_packages")
if os.path.exists("package-lock.yml"):
os.remove("package-lock.yml")
def test_add_package_with_warn_unpinned_in_yaml(self, clean_start):
"""Test that adding a package works when packages.yml contains warn-unpinned: false"""
# Before the fix, this would raise: TypeError: argument of type 'bool' is not iterable
# This matches the exact scenario from issue #9104
run_dbt(["deps", "--add-package", "dbt-labs/dbt_utils@1.0.0"])
with open("packages.yml") as fp:
contents = fp.read()
# Verify both packages are present
assert "dbt_amplitude" in contents or "fivetran/dbt_amplitude" in contents
assert "dbt-labs/dbt_utils" in contents or "dbt_utils" in contents
# The warn-unpinned should still be there
assert "warn-unpinned:" in contents or "warn_unpinned:" in contents

241
tests/unit/deps/test_deps.py
View File

@@ -21,6 +21,7 @@ from dbt.deps.registry import RegistryUnpinnedPackage
from dbt.deps.resolver import resolve_packages
from dbt.deps.tarball import TarballUnpinnedPackage
from dbt.flags import set_from_args
from dbt.task.deps import DepsTask
from dbt.version import get_installed_version
from dbt_common.dataclass_schema import ValidationError
from dbt_common.semver import VersionSpecifier
@@ -892,3 +893,243 @@ class TestPackageSpec(unittest.TestCase):
msg = "dbt-labs was not found in the package index. Packages on the index require a namespace, e.g dbt-labs/dbt_utils"
assert msg in str(exc.exception)
class TestCheckForDuplicatePackagesWithBooleans(unittest.TestCase):
"""Unit test for check_for_duplicate_packages method with boolean values.
This is a regression test for issue #9104 where the method would fail with
"TypeError: argument of type 'bool' is not iterable" when package entries
contained boolean fields like warn-unpinned: false.
"""
def test_check_duplicate_with_warn_unpinned_false(self):
"""Test that check_for_duplicate_packages handles warn-unpinned: false"""
# Create a mock DepsTask with minimal setup
mock_args = Namespace(
add_package={"name": "audit_helper", "version": "0.9.0"}, source="hub"
)
# Mock project - we don't need a real one for this test
with mock.patch("dbt.task.deps.BaseTask.__init__"):
task = DepsTask.__new__(DepsTask)
task.args = mock_args
# Test data: packages.yml with warn-unpinned: false
packages_yml = {
"packages": [
{
"git": "https://github.com/dbt-labs/dbt-utils.git",
"revision": "1.0.0",
"warn-unpinned": False, # This is the problematic boolean
},
]
}
# This should not raise TypeError
result = task.check_for_duplicate_packages(packages_yml)
# Verify the result is returned (not None)
self.assertIsNotNone(result)
self.assertIn("packages", result)
# Original package should still be there (no duplicate)
self.assertEqual(len(result["packages"]), 1)
def test_check_duplicate_with_warn_unpinned_true(self):
"""Test that check_for_duplicate_packages handles warn-unpinned: true"""
mock_args = Namespace(
add_package={"name": "audit_helper", "version": "0.9.0"}, source="hub"
)
with mock.patch("dbt.task.deps.BaseTask.__init__"):
task = DepsTask.__new__(DepsTask)
task.args = mock_args
packages_yml = {
"packages": [
{
"git": "https://github.com/dbt-labs/dbt-utils.git",
"revision": "1.0.0",
"warn-unpinned": True, # Another boolean value
},
]
}
# This should not raise TypeError
result = task.check_for_duplicate_packages(packages_yml)
self.assertIsNotNone(result)
self.assertEqual(len(result["packages"]), 1)
def test_check_duplicate_with_subdirectory_and_warn_unpinned(self):
"""Test with multiple non-string values (subdirectory string + warn-unpinned bool)"""
mock_args = Namespace(
add_package={"name": "audit_helper", "version": "0.9.0"}, source="hub"
)
with mock.patch("dbt.task.deps.BaseTask.__init__"):
task = DepsTask.__new__(DepsTask)
task.args = mock_args
packages_yml = {
"packages": [
{
"git": "https://github.com/dbt-labs/dbt-utils.git",
"revision": "1.0.0",
"subdirectory": "some_dir",
"warn-unpinned": False,
},
]
}
# This should not raise TypeError
result = task.check_for_duplicate_packages(packages_yml)
self.assertIsNotNone(result)
self.assertEqual(len(result["packages"]), 1)
def test_check_duplicate_detects_git_match(self):
"""Test that duplicate detection still works for git packages"""
mock_args = Namespace(add_package={"name": "dbt-utils", "version": "1.1.0"}, source="git")
with mock.patch("dbt.task.deps.BaseTask.__init__"):
task = DepsTask.__new__(DepsTask)
task.args = mock_args
packages_yml = {
"packages": [
{
"git": "https://github.com/dbt-labs/dbt-utils.git",
"revision": "1.0.0",
"warn-unpinned": False,
},
]
}
# The package name "dbt-utils" should match in the git URL
with mock.patch("dbt_common.events.functions.fire_event"):
result = task.check_for_duplicate_packages(packages_yml)
# The duplicate should be removed
self.assertIsNotNone(result)
self.assertEqual(len(result["packages"]), 0)
def test_check_duplicate_with_hub_package(self):
"""Test with hub package (which don't have warn-unpinned)"""
mock_args = Namespace(
add_package={"name": "another_package", "version": "1.0.0"}, source="hub"
)
with mock.patch("dbt.task.deps.BaseTask.__init__"):
task = DepsTask.__new__(DepsTask)
task.args = mock_args
packages_yml = {
"packages": [
{
"package": "dbt-labs/dbt_utils",
"version": "1.0.0",
},
]
}
# This should work fine
result = task.check_for_duplicate_packages(packages_yml)
self.assertIsNotNone(result)
self.assertEqual(len(result["packages"]), 1)
def test_check_duplicate_with_mixed_package_types(self):
"""Test with mixed package types (hub + git with warn-unpinned)"""
mock_args = Namespace(
add_package={"name": "audit_helper", "version": "0.9.0"}, source="hub"
)
with mock.patch("dbt.task.deps.BaseTask.__init__"):
task = DepsTask.__new__(DepsTask)
task.args = mock_args
packages_yml = {
"packages": [
{
"package": "dbt-labs/dbt_utils",
"version": "1.0.0",
},
{
"git": "https://github.com/example/some-package.git",
"revision": "0.5.0",
"warn-unpinned": False,
},
]
}
# This should not raise TypeError
result = task.check_for_duplicate_packages(packages_yml)
self.assertIsNotNone(result)
# Both packages should remain (no duplicates)
self.assertEqual(len(result["packages"]), 2)
def test_check_duplicate_cross_source_hub_removes_git(self):
"""Test that adding a hub package removes a git package with the same name"""
# When adding "dbt-labs/dbt_utils", it should match git URLs containing "dbt_utils"
# Note: The full package name "dbt-labs/dbt_utils" is what gets checked
mock_args = Namespace(
add_package={"name": "dbt-labs/dbt_utils", "version": "1.0.0"}, source="hub"
)
with mock.patch("dbt.task.deps.BaseTask.__init__"):
task = DepsTask.__new__(DepsTask)
task.args = mock_args
packages_yml = {
"packages": [
{
"git": "https://github.com/dbt-labs/dbt_utils.git",
"revision": "1.0.0",
"warn-unpinned": False,
},
]
}
# The hub package name "dbt-labs/dbt_utils" should match in the git URL
# because "dbt_utils" appears in both
with mock.patch("dbt_common.events.functions.fire_event"):
result = task.check_for_duplicate_packages(packages_yml)
# The git package should be removed (cross-source duplicate)
self.assertIsNotNone(result)
self.assertEqual(len(result["packages"]), 0)
def test_check_duplicate_multiple_matches(self):
"""Test that all duplicate packages are removed, not just the first one"""
# When adding "dbt-labs/dbt_utils", it should match any identifier containing "dbt_utils"
mock_args = Namespace(
add_package={"name": "dbt-labs/dbt_utils", "version": "1.0.0"}, source="hub"
)
with mock.patch("dbt.task.deps.BaseTask.__init__"):
task = DepsTask.__new__(DepsTask)
task.args = mock_args
packages_yml = {
"packages": [
{
"git": "https://github.com/dbt-labs/dbt_utils.git",
"revision": "0.9.0",
},
{
"git": "https://github.com/fivetran/dbt_amplitude",
"revision": "1.0.0",
},
{
"package": "dbt-labs/dbt_utils", # Exact match
"version": "0.8.0",
},
]
}
# Should remove both packages that contain "dbt_utils"
with mock.patch("dbt_common.events.functions.fire_event"):
result = task.check_for_duplicate_packages(packages_yml)
# Only the dbt_amplitude package should remain
self.assertIsNotNone(result)
self.assertEqual(len(result["packages"]), 1)
self.assertIn("dbt_amplitude", result["packages"][0]["git"])