From 13dd72029f5ad4f9979e43aeeeba2bc890697431 Mon Sep 17 00:00:00 2001 From: Jacob Beck Date: Wed, 17 Apr 2019 10:16:52 -0600 Subject: [PATCH 1/6] run-operation fixes Make dbt run-operation actually function at all with the RPC changes On exceptions that occur outside of actual macro execution, catch them and return failure appropriately --- core/dbt/task/run_operation.py | 34 ++++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/core/dbt/task/run_operation.py b/core/dbt/task/run_operation.py index 6ea5d91bd98..ef3a899eccb 100644 --- a/core/dbt/task/run_operation.py +++ b/core/dbt/task/run_operation.py @@ -1,6 +1,5 @@ from dbt.logger import GLOBAL_LOGGER as logger - -from dbt.task.base import BaseTask +from dbt.task.base import ConfiguredTask from dbt.adapters.factory import get_adapter from dbt.loader import GraphLoader @@ -9,7 +8,7 @@ import dbt.exceptions -class RunOperationTask(BaseTask): +class RunOperationTask(ConfiguredTask): def _get_macro_parts(self): macro_name = self.args.macro if '.' in macro_name: @@ -22,7 +21,7 @@ def _get_macro_parts(self): def _get_kwargs(self): return dbt.utils.parse_cli_vars(self.args.args) - def run(self): + def run_unsafe(self): manifest = GraphLoader.load_all(self.config) adapter = get_adapter(self.config) @@ -33,8 +32,31 @@ def run(self): macro_name, project=package_name, kwargs=macro_kwargs, - manifest=manifest, - connection_name="macro_{}".format(macro_name) + manifest=manifest ) return res + + def run(self): + try: + result = self.run_unsafe() + except dbt.exceptions.Exception as exc: + logger.error( + 'Encountered a dbt exception while running a macro: {}' + .format(exc) + ) + logger.debug('', exc_info=True) + return False, None + except Exception as exc: + logger.error( + 'Encountered an uncaught exception while running a macro: {}' + .format(exc) + ) + logger.debug('', exc_info=True) + return False, None + else: + return True, result + + def interpret_results(self, results): + success, _ = results + return success From a72a4e1fcb708fc42f3813a24717c7d856408ce4 Mon Sep 17 00:00:00 2001 From: Jacob Beck Date: Wed, 17 Apr 2019 11:26:14 -0600 Subject: [PATCH 2/6] Acquire a connection before executing the macro, and commit after --- core/dbt/task/run_operation.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/core/dbt/task/run_operation.py b/core/dbt/task/run_operation.py index ef3a899eccb..c321e82fffb 100644 --- a/core/dbt/task/run_operation.py +++ b/core/dbt/task/run_operation.py @@ -28,12 +28,14 @@ def run_unsafe(self): package_name, macro_name = self._get_macro_parts() macro_kwargs = self._get_kwargs() - res = adapter.execute_macro( - macro_name, - project=package_name, - kwargs=macro_kwargs, - manifest=manifest - ) + with adapter.connection_named('macro_{}'.format(macro_name)): + res = adapter.execute_macro( + macro_name, + project=package_name, + kwargs=macro_kwargs, + manifest=manifest + ) + adapter.commit_if_has_connection() return res From 5b74c58a43a5c32217773c0e9d7bcd8359d7ed60 Mon Sep 17 00:00:00 2001 From: Jacob Beck Date: Wed, 17 Apr 2019 11:26:18 -0600 Subject: [PATCH 3/6] add tests --- .../macros/happy_macros.sql | 16 ++++++ .../macros/sad_macros.sql | 7 +++ .../test_run_operations.py | 52 +++++++++++++++++++ 3 files changed, 75 insertions(+) create mode 100644 test/integration/044_run_operations_test/macros/happy_macros.sql create mode 100644 test/integration/044_run_operations_test/macros/sad_macros.sql create mode 100644 test/integration/044_run_operations_test/test_run_operations.py diff --git a/test/integration/044_run_operations_test/macros/happy_macros.sql b/test/integration/044_run_operations_test/macros/happy_macros.sql new file mode 100644 index 00000000000..3df736b0cd4 --- /dev/null +++ b/test/integration/044_run_operations_test/macros/happy_macros.sql @@ -0,0 +1,16 @@ +{% macro no_args() %} + {% if execute %} + {% call statement() %} + create table "{{ schema }}"."no_args" (id int) + {% endcall %} + {% endif %} +{% endmacro %} + + +{% macro table_name_args(table_name) %} + {% if execute %} + {% call statement() %} + create table "{{ schema }}"."{{ table_name }}" (id int) + {% endcall %} + {% endif %} +{% endmacro %} diff --git a/test/integration/044_run_operations_test/macros/sad_macros.sql b/test/integration/044_run_operations_test/macros/sad_macros.sql new file mode 100644 index 00000000000..4f2c80bc40f --- /dev/null +++ b/test/integration/044_run_operations_test/macros/sad_macros.sql @@ -0,0 +1,7 @@ +{% macro syntax_error() %} + {% if execute %} + {% call statement() %} + select NOPE NOT A VALID QUERY + {% endcall %} + {% endif %} +{% endmacro %} diff --git a/test/integration/044_run_operations_test/test_run_operations.py b/test/integration/044_run_operations_test/test_run_operations.py new file mode 100644 index 00000000000..2708ae84e45 --- /dev/null +++ b/test/integration/044_run_operations_test/test_run_operations.py @@ -0,0 +1,52 @@ +from test.integration.base import DBTIntegrationTest, use_profile +import yaml + + +class TestOperations(DBTIntegrationTest): + @property + def schema(self): + return "run_operations_044" + + @property + def models(self): + return "test/integration/044_run_operations_test/models" + + @property + def project_config(self): + return { + "macro-paths": ['test/integration/044_run_operations_test/macros'], + } + + def run_operation(self, macro, expect_pass=True, extra_args=None, **kwargs): + args = ['run-operation'] + if macro: + args.extend(('--macro', macro)) + if kwargs: + args.extend(('--args', yaml.safe_dump(kwargs))) + if extra_args: + args.extend(extra_args) + return self.run_dbt(args, expect_pass=expect_pass) + + @use_profile('postgres') + def test__postgres_macro_noargs(self): + self.run_operation('no_args') + self.assertTableDoesExist('no_args') + + @use_profile('postgres') + def test__postgres_macro_args(self): + self.run_operation('table_name_args', table_name='my_fancy_table') + self.assertTableDoesExist('my_fancy_table') + + @use_profile('postgres') + def test__postgres_macro_exception(self): + self.run_operation('syntax_error', False) + + @use_profile('postgres') + def test__postgres_macro_missing(self): + self.run_operation('this_macro_does_not_exist', False) + + @use_profile('postgres') + def test__postgres_cannot_connect(self): + self.run_operation('no_args', + extra_args=['--target', 'noaccess'], + expect_pass=False) From 473078986ca7905190bbf20ec3422e24e3d56c20 Mon Sep 17 00:00:00 2001 From: Jacob Beck Date: Thu, 18 Apr 2019 09:05:23 -0600 Subject: [PATCH 4/6] PR feedback: run_unsafe -> _run_unsafe --- core/dbt/task/run_operation.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/dbt/task/run_operation.py b/core/dbt/task/run_operation.py index c321e82fffb..10fe0b1e04a 100644 --- a/core/dbt/task/run_operation.py +++ b/core/dbt/task/run_operation.py @@ -21,7 +21,7 @@ def _get_macro_parts(self): def _get_kwargs(self): return dbt.utils.parse_cli_vars(self.args.args) - def run_unsafe(self): + def _run_unsafe(self): manifest = GraphLoader.load_all(self.config) adapter = get_adapter(self.config) @@ -41,7 +41,7 @@ def run_unsafe(self): def run(self): try: - result = self.run_unsafe() + result = self._run_unsafe() except dbt.exceptions.Exception as exc: logger.error( 'Encountered a dbt exception while running a macro: {}' From 08c5f9aed868b5b6a0b373a5c545ff7db506ce9d Mon Sep 17 00:00:00 2001 From: Jacob Beck Date: Sun, 21 Apr 2019 16:43:29 -0600 Subject: [PATCH 5/6] no automatic transactions --- core/dbt/task/run_operation.py | 5 ++--- .../044_run_operations_test/macros/happy_macros.sql | 10 ++++++---- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/core/dbt/task/run_operation.py b/core/dbt/task/run_operation.py index 10fe0b1e04a..5604563190b 100644 --- a/core/dbt/task/run_operation.py +++ b/core/dbt/task/run_operation.py @@ -35,7 +35,6 @@ def _run_unsafe(self): kwargs=macro_kwargs, manifest=manifest ) - adapter.commit_if_has_connection() return res @@ -44,14 +43,14 @@ def run(self): result = self._run_unsafe() except dbt.exceptions.Exception as exc: logger.error( - 'Encountered a dbt exception while running a macro: {}' + 'Encountered an error while running operation: {}' .format(exc) ) logger.debug('', exc_info=True) return False, None except Exception as exc: logger.error( - 'Encountered an uncaught exception while running a macro: {}' + 'Encountered an uncaught exception while running operation: {}' .format(exc) ) logger.debug('', exc_info=True) diff --git a/test/integration/044_run_operations_test/macros/happy_macros.sql b/test/integration/044_run_operations_test/macros/happy_macros.sql index 3df736b0cd4..adee6e528b4 100644 --- a/test/integration/044_run_operations_test/macros/happy_macros.sql +++ b/test/integration/044_run_operations_test/macros/happy_macros.sql @@ -1,7 +1,8 @@ {% macro no_args() %} {% if execute %} - {% call statement() %} - create table "{{ schema }}"."no_args" (id int) + {% call statement(auto_begin=True) %} + create table "{{ schema }}"."no_args" (id int); + commit; {% endcall %} {% endif %} {% endmacro %} @@ -9,8 +10,9 @@ {% macro table_name_args(table_name) %} {% if execute %} - {% call statement() %} - create table "{{ schema }}"."{{ table_name }}" (id int) + {% call statement(auto_begin=True) %} + create table "{{ schema }}"."{{ table_name }}" (id int); + commit; {% endcall %} {% endif %} {% endmacro %} From ca02a5851918bf0325b9b3688b4abadf5e1cf5df Mon Sep 17 00:00:00 2001 From: Jacob Beck Date: Tue, 23 Apr 2019 20:08:07 -0600 Subject: [PATCH 6/6] PR feedback: Add a clear_transaction call --- core/dbt/task/run_operation.py | 1 + .../044_run_operations_test/macros/happy_macros.sql | 6 ++++++ test/integration/044_run_operations_test/models/model.sql | 1 + .../044_run_operations_test/test_run_operations.py | 6 ++++++ 4 files changed, 14 insertions(+) create mode 100644 test/integration/044_run_operations_test/models/model.sql diff --git a/core/dbt/task/run_operation.py b/core/dbt/task/run_operation.py index 5604563190b..fe90649d1e0 100644 --- a/core/dbt/task/run_operation.py +++ b/core/dbt/task/run_operation.py @@ -29,6 +29,7 @@ def _run_unsafe(self): macro_kwargs = self._get_kwargs() with adapter.connection_named('macro_{}'.format(macro_name)): + adapter.clear_transaction() res = adapter.execute_macro( macro_name, project=package_name, diff --git a/test/integration/044_run_operations_test/macros/happy_macros.sql b/test/integration/044_run_operations_test/macros/happy_macros.sql index adee6e528b4..6170ebc7657 100644 --- a/test/integration/044_run_operations_test/macros/happy_macros.sql +++ b/test/integration/044_run_operations_test/macros/happy_macros.sql @@ -16,3 +16,9 @@ {% endcall %} {% endif %} {% endmacro %} + +{% macro vacuum(table_name) %} + {% call statement(auto_begin=false) %} + vacuum "{{ schema }}"."{{ table_name }}" + {% endcall %} +{% endmacro %} diff --git a/test/integration/044_run_operations_test/models/model.sql b/test/integration/044_run_operations_test/models/model.sql new file mode 100644 index 00000000000..43258a71464 --- /dev/null +++ b/test/integration/044_run_operations_test/models/model.sql @@ -0,0 +1 @@ +select 1 as id diff --git a/test/integration/044_run_operations_test/test_run_operations.py b/test/integration/044_run_operations_test/test_run_operations.py index 2708ae84e45..c66de6d8af5 100644 --- a/test/integration/044_run_operations_test/test_run_operations.py +++ b/test/integration/044_run_operations_test/test_run_operations.py @@ -50,3 +50,9 @@ def test__postgres_cannot_connect(self): self.run_operation('no_args', extra_args=['--target', 'noaccess'], expect_pass=False) + + @use_profile('postgres') + def test__postgres_vacuum(self): + self.run_dbt(['run']) + # this should succeed + self.run_operation('vacuum', table_name='model')