Skip to content

Commit

Permalink
run hooks in correct order, fixes #590 (#601)
Browse files Browse the repository at this point in the history
* run hooks in correct order, fixes #590

* add tests

* fix tests

* pep8
  • Loading branch information
drewbanin authored Dec 2, 2017
1 parent ed60db0 commit 33da2db
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 8 deletions.
1 change: 1 addition & 0 deletions dbt/contracts/graph/parsed.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
hook_contract = Schema({
Required('sql'): basestring,
Required('transaction'): bool,
Required('index'): int,
})

config_contract = Schema({
Expand Down
3 changes: 2 additions & 1 deletion dbt/contracts/graph/unparsed.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from voluptuous import Schema, Required, All, Any, Length
from voluptuous import Schema, Required, All, Any, Length, Optional

from dbt.compat import basestring
from dbt.contracts.common import validate_with
Expand All @@ -15,6 +15,7 @@
Required('path'): basestring,
Required('original_file_path'): basestring,
Required('raw_sql'): basestring,
Optional('index'): int,
})

unparsed_node_contract = unparsed_base_contract.extend({
Expand Down
5 changes: 3 additions & 2 deletions dbt/hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,13 @@ def _parse_hook_to_dict(hook_string):
return hook_dict


def get_hook_dict(hook):
def get_hook_dict(hook, index):
if isinstance(hook, dict):
hook_dict = hook
else:
hook_dict = _parse_hook_to_dict(to_string(hook))

hook_dict['index'] = index
return hook_dict


Expand All @@ -36,5 +37,5 @@ def get_hooks(model, hook_key):
if not isinstance(hooks, (list, tuple)):
hooks = [hooks]

wrapped = [get_hook_dict(hook) for hook in hooks]
wrapped = [get_hook_dict(hook, i) for i, hook in enumerate(hooks)]
return wrapped
6 changes: 4 additions & 2 deletions dbt/node_runners.py
Original file line number Diff line number Diff line change
Expand Up @@ -291,11 +291,13 @@ def run_hooks(cls, project, adapter, flat_graph, hook_type):
model_name = compiled.get('name')
statement = compiled['wrapped_sql']

hook_dict = dbt.hooks.get_hook_dict(statement)
hook_index = hook.get('index', len(hooks))
hook_dict = dbt.hooks.get_hook_dict(statement, index=hook_index)
compiled_hooks.append(hook_dict)

for hook in compiled_hooks:
ordered_hooks = sorted(compiled_hooks, key=lambda h: h.get('index', 0))

for hook in ordered_hooks:
if dbt.flags.STRICT_MODE:
dbt.contracts.graph.parsed.validate_hook(hook)

Expand Down
3 changes: 2 additions & 1 deletion dbt/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,8 @@ def load_and_parse_run_hook_type(root_project, all_projects, hook_type,
'path': hook_path,
'original_file_path': hook_path,
'package_name': project_name,
'raw_sql': hook
'raw_sql': hook,
'index': i
})

tags = {hook_type}
Expand Down
19 changes: 17 additions & 2 deletions test/integration/014_hook_tests/test_run_hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,19 @@ def project_config(self):
return {
'macro-paths': ['test/integration/014_hook_tests/macros'],

"on-run-start": "{{ custom_run_hook('start', target, run_started_at, invocation_id) }}",
"on-run-end": "{{ custom_run_hook('end', target, run_started_at, invocation_id) }}",
# The create and drop table statements here validate that these hooks run
# in the same order that they are defined. Drop before create is an error.
# Also check that the table does not exist below.
"on-run-start": [
"{{ custom_run_hook('start', target, run_started_at, invocation_id) }}",
"create table {{ target.schema }}.start_hook_order_test ( id int )",
"drop table {{ target.schema }}.start_hook_order_test",
],
"on-run-end": [
"{{ custom_run_hook('end', target, run_started_at, invocation_id) }}",
"create table {{ target.schema }}.end_hook_order_test ( id int )",
"drop table {{ target.schema }}.end_hook_order_test",
]
}

@property
Expand Down Expand Up @@ -76,3 +87,7 @@ def test_pre_and_post_run_hooks(self):

self.check_hooks('start')
self.check_hooks('end')


self.assertTableDoesNotExist("start_hook_order_test")
self.assertTableDoesNotExist("end_hook_order_test")

0 comments on commit 33da2db

Please sign in to comment.