fix statements in on run start hooks #693
Conversation
|
@cmcarthur nice! This should also fix the issue with connections being closed in on-run-end hooks, right? |
|
@drewbanin yep, that's right. if you have a test case for that, can you run this branch and see if it fixes it? |
|
@cmcarthur I don't have a test case handy -- let me see if I can whip one up |
dbt/node_runners.py
Outdated
| auto_begin=False) | ||
| adapter.release_connection(profile, conn_name) | ||
|
|
||
| if len(sql) > 0: |
There was a problem hiding this comment.
can we do sql.strip() here? Unsure if it will make any difference, but I know sometimes people put conditional statements into hooks, and the else clause might contain some empty space
There was a problem hiding this comment.
@cmcarthur this LGTM if you've tested on Redshift + Snowflake with and without hooks!
dbt/node_runners.py
Outdated
| # configuring psycopg2 (and other adapters?) to ensure that a | ||
| # transaction is only created if dbt initiates it. | ||
| conn_name = adapter.clear_transaction(profile) | ||
| adapter.clear_transaction(profile) |
There was a problem hiding this comment.
@cmcarthur this will use the master connection, right? I think this can still cause issues with the on-run-end hook after a long dbt run. Do we need to clear this transaction here?
dbt/node_runners.py
Outdated
|
|
||
| ordered_hooks = sorted(compiled_hooks, key=lambda h: h.get('index', 0)) | ||
|
|
||
| for hook in ordered_hooks: | ||
| model_name = compiled.get('name') |
There was a problem hiding this comment.
this should be hook, not compiled I think
…statements-in-on-run-start-hooks
|
This also fixes #620 |
statements don't really work in on run start hooks, because the connection name used doesn't match. there's some code that uses
masterand some that uses the node name. this makes everything use the node name, and releases the connection after compilation in case a statement is run.