Skip to content

Commit

Permalink
Merge pull request #1453 from fishtown-analytics/feature/warn-on-unpi…
Browse files Browse the repository at this point in the history
…nned-packages

Warn on unpinned git packages (#1446)
  • Loading branch information
beckjake authored May 10, 2019
2 parents 210cf43 + ea88259 commit d502b33
Show file tree
Hide file tree
Showing 10 changed files with 75 additions and 13 deletions.
3 changes: 3 additions & 0 deletions core/dbt/contracts/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,9 @@ class Project(APIObject):
'items': {'type': 'string'},
'description': 'The git revision to use, if it is not tip',
},
'warn-unpinned': {
'type': 'boolean',
}
},
'required': ['git'],
}
Expand Down
21 changes: 20 additions & 1 deletion core/dbt/task/deps.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from dbt.compat import basestring
from dbt.logger import GLOBAL_LOGGER as logger
from dbt.semver import VersionSpecifier, UnboundedVersionSpecifier
from dbt.ui import printer
from dbt.utils import AttrDict
from dbt.api.object import APIObject
from dbt.contracts.project import LOCAL_PACKAGE_CONTRACT, \
Expand All @@ -25,6 +26,7 @@

DOWNLOADS_PATH = None
REMOVE_DOWNLOADS = False
PIN_PACKAGE_URL = 'https://docs.getdbt.com/docs/package-management#section-specifying-package-versions' # noqa


def _initialize_downloads():
Expand Down Expand Up @@ -215,6 +217,8 @@ class GitPackage(Package):
SCHEMA = GIT_PACKAGE_CONTRACT

def __init__(self, *args, **kwargs):
if 'warn_unpinned' in kwargs:
kwargs['warn-unpinned'] = kwargs.pop('warn_unpinned')
super(GitPackage, self).__init__(*args, **kwargs)
self._checkout_name = hashlib.md5(six.b(self.git)).hexdigest()
self.version = self._contents.get('revision')
Expand Down Expand Up @@ -252,8 +256,12 @@ def nice_version_name(self):
return "revision {}".format(self.version_name())

def incorporate(self, other):
# if one is False, make both be False.
warn_unpinned = self.warn_unpinned and other.warn_unpinned

return GitPackage(git=self.git,
revision=(self.version + other.version))
revision=(self.version + other.version),
warn_unpinned=warn_unpinned)

def _resolve_version(self):
requested = set(self.version)
Expand All @@ -263,6 +271,10 @@ def _resolve_version(self):
'{} contains: {}'.format(self.git, requested))
self.version = requested.pop()

@property
def warn_unpinned(self):
return self.get('warn-unpinned', True)

def _checkout(self, project):
"""Performs a shallow clone of the repository into the downloads
directory. This function can be called repeatedly. If the project has
Expand All @@ -287,6 +299,13 @@ def _checkout(self, project):

def _fetch_metadata(self, project):
path = self._checkout(project)
if self.version[0] == 'master' and self.warn_unpinned:
dbt.exceptions.warn_or_error(
'The git package "{}" is not pinned.\n\tThis can introduce '
'breaking changes into your project without warning!\n\nSee {}'
.format(self.git, PIN_PACKAGE_URL),
log_fmt=printer.yellow('WARNING: {}')
)
loaded = project.from_project_root(path, {})
return ProjectPackageMetadata(loaded)

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import os
from test.integration.base import DBTIntegrationTest, use_profile
from dbt.exceptions import CompilationException


class TestSimpleDependency(DBTIntegrationTest):
Expand All @@ -21,13 +22,14 @@ def packages_config(self):
return {
"packages": [
{
'git': 'https://github.com/fishtown-analytics/dbt-integration-project'
'git': 'https://github.com/fishtown-analytics/dbt-integration-project',
'warn-unpinned': False,
}
]
}

@use_profile('postgres')
def test_simple_dependency(self):
def test_postgres_simple_dependency(self):
self.run_dbt(["deps"])
results = self.run_dbt(["run"])
self.assertEqual(len(results), 4)
Expand All @@ -49,7 +51,7 @@ def test_simple_dependency(self):
self.assertTablesEqual("seed","incremental")

@use_profile('postgres')
def test_simple_dependency_with_models(self):
def test_postgres_simple_dependency_with_models(self):
self.run_dbt(["deps"])
results = self.run_dbt(["run", '--models', 'view_model+'])
self.assertEqual(len(results), 2)
Expand All @@ -66,6 +68,37 @@ def test_simple_dependency_with_models(self):
self.assertEqual(created_models['view_summary'], 'view')


class TestSimpleDependencyUnpinned(DBTIntegrationTest):
def setUp(self):
DBTIntegrationTest.setUp(self)
self.run_sql_file("test/integration/006_simple_dependency_test/seed.sql")

@property
def schema(self):
return "simple_dependency_006"

@property
def models(self):
return "test/integration/006_simple_dependency_test/models"

@property
def packages_config(self):
return {
"packages": [
{
'git': 'https://github.com/fishtown-analytics/dbt-integration-project',
'warn-unpinned': True,
}
]
}

@use_profile('postgres')
def test_postgres_simple_dependency(self):
self.run_dbt(['deps'], strict=False)
with self.assertRaises(CompilationException):
self.run_dbt(["deps"])


class TestSimpleDependencyWithDuplicates(DBTIntegrationTest):
@property
def schema(self):
Expand All @@ -81,10 +114,12 @@ def packages_config(self):
return {
"packages": [
{
'git': 'https://github.com/fishtown-analytics/dbt-integration-project'
'git': 'https://github.com/fishtown-analytics/dbt-integration-project',
'warn-unpinned': False,
},
{
'git': 'https://github.com/fishtown-analytics/dbt-integration-project.git'
'git': 'https://github.com/fishtown-analytics/dbt-integration-project.git',
'warn-unpinned': False,
}
]
}
Expand Down Expand Up @@ -147,6 +182,7 @@ def packages_config(self):
{
'git': 'https://github.com/fishtown-analytics/dbt-integration-project',
'revision': 'master',
'warn-unpinned': False,
},
]
}
Expand All @@ -168,7 +204,7 @@ def deps_run_assert_equality(self):
self.assertEqual(created_models['incremental'], 'table')

@use_profile('postgres')
def test_simple_dependency(self):
def test_postgres_simple_dependency(self):
self.deps_run_assert_equality()

self.assertTablesEqual("seed_summary","view_summary")
Expand All @@ -178,7 +214,7 @@ def test_simple_dependency(self):
self.deps_run_assert_equality()

@use_profile('postgres')
def test_empty_models_not_compiled_in_dependencies(self):
def test_postgres_empty_models_not_compiled_in_dependencies(self):
self.deps_run_assert_equality()

models = self.get_models_in_schema()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def models(self):
def packages_config(self):
return {
"packages": [
{'git': 'https://github.com/fishtown-analytics/dbt-integration-project'}
{'git': 'https://github.com/fishtown-analytics/dbt-integration-project', 'warn-unpinned': False}
]
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ def packages_config(self):
},
{
'git': 'https://github.com/fishtown-analytics/dbt-integration-project',
'warn-unpinned': False,
},
]
}
Expand Down
4 changes: 2 additions & 2 deletions test/integration/016_macro_tests/test_macros.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def models(self):
def packages_config(self):
return {
'packages': [
{'git': 'https://github.com/fishtown-analytics/dbt-integration-project'},
{'git': 'https://github.com/fishtown-analytics/dbt-integration-project', 'warn-unpinned': False}
]
}

Expand Down Expand Up @@ -92,7 +92,7 @@ def models(self):
def packages_config(self):
return {
'packages': [
{'git': 'https://github.com/fishtown-analytics/dbt-integration-project'}
{'git': 'https://github.com/fishtown-analytics/dbt-integration-project', 'warn-unpinned': False}
]
}

Expand Down
2 changes: 1 addition & 1 deletion test/integration/023_exit_codes_test/test_exit_codes.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ def models(self):
def packages_config(self):
return {
"packages": [
{'git': 'https://github.com/fishtown-analytics/dbt-integration-project'}
{'git': 'https://github.com/fishtown-analytics/dbt-integration-project', 'warn-unpinned': False}
]
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ def packages_config(self):
{
'git': 'https://github.com/fishtown-analytics/dbt-integration-project',
'revision': 'master',
'warn-unpinned': False,
},
],
}
Expand Down Expand Up @@ -139,6 +140,7 @@ def packages_config(self):
{
'git': 'https://github.com/fishtown-analytics/dbt-integration-project',
'revision': 'master',
'warn-unpinned': False,
},
],
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ def packages_config(self):
'packages': [
{
'git': 'https://github.com/fishtown-analytics/dbt-integration-project',
'warn-unpinned': False,
},
],
}
Expand Down
2 changes: 1 addition & 1 deletion test/integration/033_event_tracking_test/test_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ class TestEventTrackingSuccess(TestEventTracking):
def packages_config(self):
return {
'packages': [
{'git': 'https://github.com/fishtown-analytics/dbt-integration-project'},
{'git': 'https://github.com/fishtown-analytics/dbt-integration-project', 'warn-unpinned': False},
],
}

Expand Down

0 comments on commit d502b33

Please sign in to comment.