From 83a4562b9829b949034cb20905990f27d78104ab Mon Sep 17 00:00:00 2001 From: Jacob Beck Date: Wed, 10 Jun 2020 15:40:41 -0600 Subject: [PATCH 1/4] Make deps respect --project-dir --- CHANGELOG.md | 2 +- core/dbt/config/runtime.py | 2 +- core/dbt/task/deps.py | 9 ++++++++- core/dbt/task/rpc/deps.py | 7 +++---- .../local_dependency/dbt_project.yml | 3 +++ .../local_dependency/macros/my_test_macro.sql | 2 ++ .../models/subdir1/subdir2/model.sql | 3 +++ .../test_cli_invocation.py | 15 +++++++++++++++ 8 files changed, 36 insertions(+), 7 deletions(-) create mode 100644 test/integration/015_cli_invocation_tests/local_dependency/dbt_project.yml create mode 100644 test/integration/015_cli_invocation_tests/local_dependency/macros/my_test_macro.sql diff --git a/CHANGELOG.md b/CHANGELOG.md index bc11cf0e1bb..bdee1e6f5d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ### Fixes - dbt compile and ls no longer create schemas if they don't already exist ([#2525](https://github.com/fishtown-analytics/dbt/issues/2525), [#2528](https://github.com/fishtown-analytics/dbt/pull/2528)) - +- `dbt deps` now respects the `--project-dir` flag, so using `dbt deps --project-dir=/some/path` and then `dbt run --project-dir=/some/path` will properly find dependencies ([#2519](https://github.com/fishtown-analytics/dbt/issues/2519), [#2534](https://github.com/fishtown-analytics/dbt/pull/2534)) ## dbt 0.17.0 (June 08, 2020) diff --git a/core/dbt/config/runtime.py b/core/dbt/config/runtime.py index b5854322d3c..3cd69fd3e9f 100644 --- a/core/dbt/config/runtime.py +++ b/core/dbt/config/runtime.py @@ -353,7 +353,7 @@ def load_projects( yield project.project_name, project def _get_project_directories(self) -> Iterator[Path]: - root = Path(self.project_root) / self.modules_path + root = Path.cwd() / self.modules_path if root.exists(): for path in root.iterdir(): diff --git a/core/dbt/task/deps.py b/core/dbt/task/deps.py index 1b6cc325292..14320fbf2f4 100644 --- a/core/dbt/task/deps.py +++ b/core/dbt/task/deps.py @@ -11,7 +11,7 @@ from dbt.logger import GLOBAL_LOGGER as logger from dbt.clients import system -from dbt.task.base import BaseTask +from dbt.task.base import BaseTask, move_to_nearest_project_dir class DepsTask(BaseTask): @@ -65,3 +65,10 @@ def run(self): package_name=package.name, source_type=package.source_type(), version=package.get_version()) + + @classmethod + def from_args(cls, args): + # deps needs to move to the project directory, as it does put files + # into the modules directory + move_to_nearest_project_dir(args) + return super().from_args(args) diff --git a/core/dbt/task/rpc/deps.py b/core/dbt/task/rpc/deps.py index b6506434a45..f2138177efb 100644 --- a/core/dbt/task/rpc/deps.py +++ b/core/dbt/task/rpc/deps.py @@ -9,10 +9,9 @@ def _clean_deps(config): - modules_dir = os.path.join(config.project_root, config.modules_path) - if os.path.exists(modules_dir): - shutil.rmtree(modules_dir) - os.makedirs(modules_dir) + if os.path.exists(config.modules_path): + shutil.rmtree(config.modules_path) + os.makedirs(config.modules_path) class RemoteDepsTask( diff --git a/test/integration/015_cli_invocation_tests/local_dependency/dbt_project.yml b/test/integration/015_cli_invocation_tests/local_dependency/dbt_project.yml new file mode 100644 index 00000000000..72672e8832e --- /dev/null +++ b/test/integration/015_cli_invocation_tests/local_dependency/dbt_project.yml @@ -0,0 +1,3 @@ +name: dependency +version: '1.0.0' +config-version: 2 diff --git a/test/integration/015_cli_invocation_tests/local_dependency/macros/my_test_macro.sql b/test/integration/015_cli_invocation_tests/local_dependency/macros/my_test_macro.sql new file mode 100644 index 00000000000..85defd788ef --- /dev/null +++ b/test/integration/015_cli_invocation_tests/local_dependency/macros/my_test_macro.sql @@ -0,0 +1,2 @@ +{% macro some_macro() %} +{% endmacro %} diff --git a/test/integration/015_cli_invocation_tests/models/subdir1/subdir2/model.sql b/test/integration/015_cli_invocation_tests/models/subdir1/subdir2/model.sql index 1c8ae4dabf3..eb345d4c460 100644 --- a/test/integration/015_cli_invocation_tests/models/subdir1/subdir2/model.sql +++ b/test/integration/015_cli_invocation_tests/models/subdir1/subdir2/model.sql @@ -4,4 +4,7 @@ ) }} +{# we don't care what, just do anything that will fail without "dbt deps" #} +{% do dependency.some_macro() %} + select 1 as id diff --git a/test/integration/015_cli_invocation_tests/test_cli_invocation.py b/test/integration/015_cli_invocation_tests/test_cli_invocation.py index 4465ca3aac6..986f3728b5d 100644 --- a/test/integration/015_cli_invocation_tests/test_cli_invocation.py +++ b/test/integration/015_cli_invocation_tests/test_cli_invocation.py @@ -6,6 +6,7 @@ class ModelCopyingIntegrationTest(DBTIntegrationTest): + def _symlink_test_folders(self): # dbt's normal symlink behavior breaks this test, so special-case it for entry in os.listdir(self.test_original_source_path): @@ -13,9 +14,20 @@ def _symlink_test_folders(self): tst = os.path.join(self.test_root_dir, entry) if entry == 'models': shutil.copytree(src, tst) + elif entry == 'local_dependency': + continue elif os.path.isdir(entry) or entry.endswith('.sql'): os.symlink(src, tst) + @property + def packages_config(self): + path = os.path.join(self.test_original_source_path, 'local_dependency') + return { + 'packages': [{ + 'local': path, + }], + } + class TestCLIInvocation(ModelCopyingIntegrationTest): @@ -33,6 +45,7 @@ def models(self): @use_profile('postgres') def test_postgres_toplevel_dbt_run(self): + self.run_dbt(['deps']) results = self.run_dbt(['run']) self.assertEqual(len(results), 1) self.assertTablesEqual("seed", "model") @@ -40,6 +53,7 @@ def test_postgres_toplevel_dbt_run(self): @use_profile('postgres') def test_postgres_subdir_dbt_run(self): os.chdir(os.path.join(self.models, "subdir1")) + self.run_dbt(['deps']) results = self.run_dbt(['run']) self.assertEqual(len(results), 1) @@ -99,6 +113,7 @@ def models(self): @use_profile('postgres') def test_postgres_toplevel_dbt_run_with_profile_dir_arg(self): + self.run_dbt(['deps']) results = self.run_dbt(['run', '--profiles-dir', 'dbt-profile'], profiles_dir=False) self.assertEqual(len(results), 1) From e00d1ed78c4c713889694694273677d431d9b7b5 Mon Sep 17 00:00:00 2001 From: Jacob Beck Date: Thu, 11 Jun 2020 09:31:14 -0600 Subject: [PATCH 2/4] make tests drop more things at cleanup Make the unique ID a bit more collision-resistant to try to avoid collisions --- .../test_simple_snapshot.py | 8 ++ .../005_simple_seed_test/test_simple_seed.py | 10 +- .../test_local_dependency.py | 9 ++ .../test_cli_invocation.py | 8 +- .../test_custom_schema.py | 120 ++++++++++++++---- .../026_aliases_test/test_aliases.py | 9 ++ test/integration/base.py | 9 +- 7 files changed, 142 insertions(+), 31 deletions(-) diff --git a/test/integration/004_simple_snapshot_test/test_simple_snapshot.py b/test/integration/004_simple_snapshot_test/test_simple_snapshot.py index c6d3e80f7c2..fc3e1be3840 100644 --- a/test/integration/004_simple_snapshot_test/test_simple_snapshot.py +++ b/test/integration/004_simple_snapshot_test/test_simple_snapshot.py @@ -406,6 +406,7 @@ def test__bigquery__snapshot_with_new_field(self): class TestCrossDBSnapshotFiles(DBTIntegrationTest): setup_alternate_db = True + @property def schema(self): return "simple_snapshot_004" @@ -465,6 +466,13 @@ def test__bigquery__cross_snapshot(self): class TestCrossSchemaSnapshotFiles(DBTIntegrationTest): NUM_SNAPSHOT_MODELS = 1 + def setUp(self): + super().setUp() + self._created_schemas.add( + self._get_schema_fqn(self.default_database, self.target_schema()), + ) + + @property def schema(self): return "simple_snapshot_004" diff --git a/test/integration/005_simple_seed_test/test_simple_seed.py b/test/integration/005_simple_seed_test/test_simple_seed.py index f1c3cd67817..4e5bb384021 100644 --- a/test/integration/005_simple_seed_test/test_simple_seed.py +++ b/test/integration/005_simple_seed_test/test_simple_seed.py @@ -54,8 +54,11 @@ def test_postgres_simple_seed_with_drop(self): class TestSimpleSeedCustomSchema(DBTIntegrationTest): def setUp(self): - DBTIntegrationTest.setUp(self) + super().setUp() self.run_sql_file("seed.sql") + self._created_schemas.add( + self._get_schema_fqn(self.default_database, self.custom_schema_name()) + ) @property def schema(self): @@ -76,9 +79,12 @@ def project_config(self): }, } + def custom_schema_name(self): + return "{}_{}".format(self.unique_schema(), 'custom_schema') + @use_profile('postgres') def test_postgres_simple_seed_with_schema(self): - schema_name = "{}_{}".format(self.unique_schema(), 'custom_schema') + schema_name = self.custom_schema_name() results = self.run_dbt(["seed"]) self.assertEqual(len(results), 1) diff --git a/test/integration/006_simple_dependency_test/test_local_dependency.py b/test/integration/006_simple_dependency_test/test_local_dependency.py index 62a103bd71f..3691b19c5d7 100644 --- a/test/integration/006_simple_dependency_test/test_local_dependency.py +++ b/test/integration/006_simple_dependency_test/test_local_dependency.py @@ -25,6 +25,15 @@ def base_schema(self): def configured_schema(self): return self.unique_schema() + '_configured' + def setUp(self): + super().setUp() + self._created_schemas.add( + self._get_schema_fqn(self.default_database, self.base_schema()) + ) + self._created_schemas.add( + self._get_schema_fqn(self.default_database, self.configured_schema()) + ) + @property def packages_config(self): return { diff --git a/test/integration/015_cli_invocation_tests/test_cli_invocation.py b/test/integration/015_cli_invocation_tests/test_cli_invocation.py index 986f3728b5d..63ad84bf17e 100644 --- a/test/integration/015_cli_invocation_tests/test_cli_invocation.py +++ b/test/integration/015_cli_invocation_tests/test_cli_invocation.py @@ -65,8 +65,8 @@ class TestCLIInvocationWithProfilesDir(ModelCopyingIntegrationTest): def setUp(self): super().setUp() - self.run_sql("DROP SCHEMA IF EXISTS {} CASCADE;".format(self.custom_schema)) - self.run_sql("CREATE SCHEMA {};".format(self.custom_schema)) + self.run_sql(f"DROP SCHEMA IF EXISTS {self.custom_schema} CASCADE;") + self.run_sql(f"CREATE SCHEMA {self.custom_schema};") # the test framework will remove this in teardown for us. if not os.path.exists('./dbt-profile'): @@ -77,6 +77,10 @@ def setUp(self): self.run_sql_file("seed_custom.sql") + def tearDown(self): + self.run_sql(f"DROP SCHEMA IF EXISTS {self.custom_schema} CASCADE;") + super().tearDown() + def custom_profile_config(self): return { 'config': { diff --git a/test/integration/024_custom_schema_test/test_custom_schema.py b/test/integration/024_custom_schema_test/test_custom_schema.py index 724ac5c45e9..f1ae44cbe89 100644 --- a/test/integration/024_custom_schema_test/test_custom_schema.py +++ b/test/integration/024_custom_schema_test/test_custom_schema.py @@ -2,6 +2,14 @@ class TestCustomSchema(DBTIntegrationTest): + def setUp(self): + super().setUp() + self._created_schemas.add( + self._get_schema_fqn(self.default_database, self.v2_schema()) + ) + self._created_schemas.add( + self._get_schema_fqn(self.default_database, self.xf_schema()) + ) @property def schema(self): @@ -11,6 +19,12 @@ def schema(self): def models(self): return "models" + def v2_schema(self): + return f"{self.unique_schema()}_custom" + + def xf_schema(self): + return f"{self.unique_schema()}_test" + @use_profile('postgres') def test__postgres__custom_schema_no_prefix(self): self.run_sql_file("seed.sql") @@ -19,19 +33,28 @@ def test__postgres__custom_schema_no_prefix(self): self.assertEqual(len(results), 3) schema = self.unique_schema() - v2_schema = "{}_custom".format(schema) - xf_schema = "{}_test".format(schema) self.assertTablesEqual("seed", "view_1") - self.assertTablesEqual("seed", "view_2", schema, v2_schema) - self.assertTablesEqual("agg", "view_3", schema, xf_schema) + self.assertTablesEqual("seed", "view_2", schema, self.v2_schema()) + self.assertTablesEqual("agg", "view_3", schema, self.xf_schema()) class TestCustomProjectSchemaWithPrefix(DBTIntegrationTest): + def setUp(self): + super().setUp() + self._created_schemas.add( + self._get_schema_fqn(self.default_database, self.v1_schema()) + ) + self._created_schemas.add( + self._get_schema_fqn(self.default_database, self.v2_schema()) + ) + self._created_schemas.add( + self._get_schema_fqn(self.default_database, self.xf_schema()) + ) @property def schema(self): - return "custom_schema_024" + return "custom_prefix_024" @property def models(self): @@ -66,6 +89,15 @@ def project_config(self): }, } + def v1_schema(self): + return f"{self.unique_schema()}_dbt_test" + + def v2_schema(self): + return f"{self.unique_schema()}_custom" + + def xf_schema(self): + return f"{self.unique_schema()}_test" + def _list_schemas(self): with self.get_connection(): return set(self.adapter.list_schemas(self.default_database)) @@ -79,10 +111,7 @@ def assert_schemas_not_created(self, expected): @use_profile('postgres') def test__postgres__custom_schema_with_prefix(self): schema = self.unique_schema() - v1_schema = f"{schema}_dbt_test" - v2_schema = f"{schema}_custom" - xf_schema = f"{schema}_test" - new_schemas = {v1_schema, v2_schema, xf_schema} + new_schemas = {self.v1_schema(), self.v2_schema(), self.xf_schema()} self.assert_schemas_not_created(new_schemas) @@ -97,16 +126,27 @@ def test__postgres__custom_schema_with_prefix(self): self.assertEqual(len(results), 3) self.assert_schemas_created(new_schemas) - self.assertTablesEqual("seed", "view_1", schema, v1_schema) - self.assertTablesEqual("seed", "view_2", schema, v2_schema) - self.assertTablesEqual("agg", "view_3", schema, xf_schema) + self.assertTablesEqual("seed", "view_1", schema, self.v1_schema()) + self.assertTablesEqual("seed", "view_2", schema, self.v2_schema()) + self.assertTablesEqual("agg", "view_3", schema, self.xf_schema()) class TestCustomProjectSchemaWithPrefixSnowflake(DBTIntegrationTest): + def setUp(self): + super().setUp() + self._created_schemas.add( + self._get_schema_fqn(self.default_database, self.v1_schema()) + ) + self._created_schemas.add( + self._get_schema_fqn(self.default_database, self.v2_schema()) + ) + self._created_schemas.add( + self._get_schema_fqn(self.default_database, self.xf_schema()) + ) @property def schema(self): - return "custom_schema_024" + return "sf_custom_prefix_024" @property def models(self): @@ -121,6 +161,15 @@ def project_config(self): } } + def v1_schema(self): + return f"{self.unique_schema()}_DBT_TEST" + + def v2_schema(self): + return f"{self.unique_schema()}_CUSTOM" + + def xf_schema(self): + return f"{self.unique_schema()}_TEST" + @use_profile('snowflake') def test__snowflake__custom_schema_with_prefix(self): self.run_sql_file("seed.sql") @@ -129,20 +178,29 @@ def test__snowflake__custom_schema_with_prefix(self): self.assertEqual(len(results), 3) schema = self.unique_schema().upper() - v1_schema = "{}_DBT_TEST".format(schema) - v2_schema = "{}_CUSTOM".format(schema) - xf_schema = "{}_TEST".format(schema) - self.assertTablesEqual("SEED", "VIEW_1", schema, v1_schema) - self.assertTablesEqual("SEED", "VIEW_2", schema, v2_schema) - self.assertTablesEqual("AGG", "VIEW_3", schema, xf_schema) + self.assertTablesEqual("SEED", "VIEW_1", schema, self.v1_schema()) + self.assertTablesEqual("SEED", "VIEW_2", schema, self.v2_schema()) + self.assertTablesEqual("AGG", "VIEW_3", schema, self.xf_schema()) class TestCustomSchemaWithCustomMacro(DBTIntegrationTest): + def setUp(self): + super().setUp() + + self._created_schemas.add( + self._get_schema_fqn(self.default_database, self.v1_schema()) + ) + self._created_schemas.add( + self._get_schema_fqn(self.default_database, self.v2_schema()) + ) + self._created_schemas.add( + self._get_schema_fqn(self.default_database, self.xf_schema()) + ) @property def schema(self): - return "custom_schema_024" + return "custom_macro_024" @property def models(self): @@ -178,6 +236,15 @@ def project_config(self): } } + def v1_schema(self): + return f"dbt_test_{self.unique_schema()}_macro" + + def v2_schema(self): + return f"custom_{self.unique_schema()}_macro" + + def xf_schema(self): + return f"test_{self.unique_schema()}_macro" + @use_profile('postgres') def test__postgres__custom_schema_from_macro(self): self.run_sql_file("seed.sql") @@ -186,17 +253,18 @@ def test__postgres__custom_schema_from_macro(self): self.assertEqual(len(results), 3) schema = self.unique_schema() - v1_schema = "dbt_test_{}_macro".format(schema) - v2_schema = "custom_{}_macro".format(schema) - xf_schema = "test_{}_macro".format(schema) - self.assertTablesEqual("seed", "view_1", schema, v1_schema) - self.assertTablesEqual("seed", "view_2", schema, v2_schema) - self.assertTablesEqual("agg", "view_3", schema, xf_schema) + self.assertTablesEqual("seed", "view_1", schema, self.v1_schema()) + self.assertTablesEqual("seed", "view_2", schema, self.v2_schema()) + self.assertTablesEqual("agg", "view_3", schema, self.xf_schema()) class TestCustomSchemaWithCustomMacroConfigs(TestCustomSchemaWithCustomMacro): + @property + def schema(self): + return "custom_macro_cfg_024" + @property def project_config(self): return { diff --git a/test/integration/026_aliases_test/test_aliases.py b/test/integration/026_aliases_test/test_aliases.py index 0ef77f04c22..fd6a3521299 100644 --- a/test/integration/026_aliases_test/test_aliases.py +++ b/test/integration/026_aliases_test/test_aliases.py @@ -85,6 +85,15 @@ def project_config(self): "macro-paths": ['macros'], } + def setUp(self): + super().setUp() + self._created_schemas.add( + self._get_schema_fqn(self.default_database, self.unique_schema() + '_schema_a') + ) + self._created_schemas.add( + self._get_schema_fqn(self.default_database, self.unique_schema() + '_schema_b') + ) + @use_profile('postgres') def test__postgres_same_alias_succeeds_in_different_schemas(self): results = self.run_dbt(['run']) diff --git a/test/integration/base.py b/test/integration/base.py index 99dec5bebed..7122d1a6c86 100644 --- a/test/integration/base.py +++ b/test/integration/base.py @@ -125,7 +125,14 @@ class DBTIntegrationTest(unittest.TestCase): CREATE_SCHEMA_STATEMENT = 'CREATE SCHEMA {}' DROP_SCHEMA_STATEMENT = 'DROP SCHEMA IF EXISTS {} CASCADE' - prefix = "test{}{:04}".format(int(time.time()), random.randint(0, 9999)) + _randint = random.randint(0, 9999) + _runtime_timedelta = (datetime.utcnow() - datetime(1970, 1, 1, 0, 0, 0)) + _runtime = ( + (int(_runtime_timedelta.total_seconds() * 1e6)) + + _runtime_timedelta.microseconds + ) + + prefix = f'test{_runtime}{_randint:04}' setup_alternate_db = False @property From 7ee6f1de392e733371b247e55c81674051ce4db2 Mon Sep 17 00:00:00 2001 From: Jacob Beck Date: Fri, 12 Jun 2020 13:00:00 -0600 Subject: [PATCH 3/4] allow relative paths in project-dir --- core/dbt/main.py | 4 ++++ .../015_cli_invocation_tests/test_cli_invocation.py | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/core/dbt/main.py b/core/dbt/main.py index 3e20e0ec376..3c6d99883f6 100644 --- a/core/dbt/main.py +++ b/core/dbt/main.py @@ -907,6 +907,10 @@ def parse_args(args, cls=DBTArgumentParser): if hasattr(parsed, 'profiles_dir'): parsed.profiles_dir = os.path.expanduser(parsed.profiles_dir) + if getattr(parsed, 'project_dir', None) is not None: + expanded_user = os.path.expanduser(parsed.project_dir) + parsed.project_dir = os.path.abspath(expanded_user) + if not hasattr(parsed, 'which'): # the user did not provide a valid subcommand. trigger the help message # and exit with a error diff --git a/test/integration/015_cli_invocation_tests/test_cli_invocation.py b/test/integration/015_cli_invocation_tests/test_cli_invocation.py index 63ad84bf17e..72c76846b24 100644 --- a/test/integration/015_cli_invocation_tests/test_cli_invocation.py +++ b/test/integration/015_cli_invocation_tests/test_cli_invocation.py @@ -155,6 +155,14 @@ def test_postgres_dbt_commands_with_randomdir_as_project_dir(self): self._run_simple_dbt_commands(workdir) os.chdir(workdir) + @use_profile('postgres') + def test_postgres_dbt_commands_with_relative_dir_as_project_dir(self): + workdir = os.getcwd() + with tempfile.TemporaryDirectory() as tmpdir: + os.chdir(tmpdir) + self._run_simple_dbt_commands(os.path.relpath(workdir, tmpdir)) + os.chdir(workdir) + def _run_simple_dbt_commands(self, project_dir): self.run_dbt(['deps', '--project-dir', project_dir]) self.run_dbt(['seed', '--project-dir', project_dir]) From e9b4c4788783ee23ce3b5e07118fb6bff6efc79a Mon Sep 17 00:00:00 2001 From: Jacob Beck Date: Mon, 15 Jun 2020 10:07:58 -0600 Subject: [PATCH 4/4] PR feedback: use the project root instead of cwd --- core/dbt/config/runtime.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/dbt/config/runtime.py b/core/dbt/config/runtime.py index 3cd69fd3e9f..b5854322d3c 100644 --- a/core/dbt/config/runtime.py +++ b/core/dbt/config/runtime.py @@ -353,7 +353,7 @@ def load_projects( yield project.project_name, project def _get_project_directories(self) -> Iterator[Path]: - root = Path.cwd() / self.modules_path + root = Path(self.project_root) / self.modules_path if root.exists(): for path in root.iterdir():