Skip to content

Commit

Permalink
Fix dbt deps failing on tarballs (#9068)
Browse files Browse the repository at this point in the history
* tarball lockfile fix

* Add changie doc for tarball deps issue

* Add integration test for ensuring tarball package specification works

This test was written _after_ the fix was commited. However, I ran this
test against main without the fix and it failed. After running the test
with the tarball fix, it passed.

* Remove unnecessary `tarball` conditional logic in `PackageConfig.validate`

We had a conditional to skip validation for a package if the package
included the `tarball` key. However, this conditional always returned
false as it was nested inside a conditional that the package had the
default `package` key, which means it's not a tarball package, but a
package package (maybe we need better differentiation here). If we need
additional validation for tarballs down the road, we should do that one
level up. At this time we have no additional validaitons to add.

* Fix typos in changie doc for tarball deps issue

* Improve tarball package test naming and add related unhappy path test

* Remove unnecessary `setUp` fixture from tarball package tests

We initially included this fixture due to copy and pasting another
test. However, this `setUp` fixture isn't actually necessary for the
tarball dependency tests.

---------

Co-authored-by: Chenyu Li <chenyu.li@dbtlabs.com>
(cherry picked from commit 017faf4)
  • Loading branch information
QMalcolm authored and github-actions[bot] committed Nov 14, 2023
1 parent 7eb6cdb commit 2fab832
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 2 deletions.
6 changes: 6 additions & 0 deletions .changes/unreleased/Fixes-20231113-114956.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Fixes
body: Fix formatting of tarball information in packages-lock.yml
time: 2023-11-13T11:49:56.437007-08:00
custom:
Author: ChenyuLInx QMalcolm
Issue: "9062"
3 changes: 1 addition & 2 deletions core/dbt/deps/tarball.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ def name(self):
def to_dict(self) -> Dict[str, str]:
return {
"tarball": self.tarball,
"version": self.version,
"package": self.package,
"name": self.package,
}

def get_version(self):
Expand Down
41 changes: 41 additions & 0 deletions tests/functional/dependencies/test_simple_dependency.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@

from pathlib import Path

from dbt.exceptions import DbtProjectError
from dbt.tests.util import (
check_relations_equal,
run_dbt,
write_config_file,
)


Expand Down Expand Up @@ -374,3 +376,42 @@ def test_deps_bad_profile(self, project):
del os.environ["PROFILE_TEST_HOST"]
run_dbt(["deps"])
run_dbt(["clean"])


class TestSimpleDependcyTarball(object):
@pytest.fixture(scope="class")
def packages(self):
return {
"packages": [
{
"tarball": "https://codeload.github.com/dbt-labs/dbt-utils/tar.gz/0.9.6",
"name": "dbt_utils",
}
]
}

def test_deps_simple_tarball_doesnt_error_out(self, project):
run_dbt(["deps"])
assert len(os.listdir("dbt_packages")) == 1


class TestBadTarballDependency(object):
def test_malformed_tarball_package_causes_exception(self, project):
# We have to specify the bad formatted package here because if we do it
# in a `packages` fixture, the test will blow up in the setup phase, meaning
# we can't appropriately catch it with a `pytest.raises`
bad_tarball_package_spec = {
"packages": [
{
"tarball": "https://codeload.github.com/dbt-labs/dbt-utils/tar.gz/0.9.6",
"version": "dbt_utils",
}
]
}
write_config_file(bad_tarball_package_spec, "packages.yml")

with pytest.raises(
DbtProjectError, match=r"The packages.yml file in this project is malformed"
) as e:
run_dbt(["deps"])
assert e is not None

0 comments on commit 2fab832

Please sign in to comment.