Support dbt Fusion#840
Conversation
kverburg
had a problem deploying
to
elementary_test_env
GitHub Actions
Error
kverburg
had a problem deploying
to
elementary_test_env
GitHub Actions
Error
kverburg
had a problem deploying
to
elementary_test_env
GitHub Actions
Error
kverburg
had a problem deploying
to
elementary_test_env
GitHub Actions
Error
kverburg
had a problem deploying
to
elementary_test_env
GitHub Actions
Error
kverburg
had a problem deploying
to
elementary_test_env
GitHub Actions
Error
kverburg
had a problem deploying
to
elementary_test_env
GitHub Actions
Error
kverburg
had a problem deploying
to
elementary_test_env
GitHub Actions
Error
kverburg
had a problem deploying
to
elementary_test_env
GitHub Actions
Error
kverburg
had a problem deploying
to
elementary_test_env
GitHub Actions
Error
WalkthroughAdds new ignore patterns to .gitignore. Modifies dbt hook macros to return simple SQL strings at run start and end. Updates models/run_results.yml to relocate deprecation metadata for compiled_sql from a top-level meta block into config.meta in two locations. Changes
Sequence Diagram(s)sequenceDiagram
participant Dbt as dbt runtime
participant Hook as on_run_start macro
Dbt->>Hook: Invoke on_run_start
Hook->>Hook: Perform existing setup logic
Hook-->>Dbt: Return SQL: select 'on-run-start'
sequenceDiagram
participant Dbt as dbt runtime
participant Hook as on_run_end macro
Dbt->>Hook: Invoke on_run_end
Hook->>Hook: Perform existing cleanup/upload logic
Hook-->>Dbt: Return SQL: select 'on-run-end'
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
👋 @kverburg |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
macros/edr/system/hooks/on_run_end.sql (1)
39-44: Clarify when to emit dummy SQL; consider scoping under execute/docs conditionCurrently, dummy_query is always returned even when execute is false or the command is docs (the internal work is skipped, but the SELECT still returns). If Fusion requires a SQL return only in execute runs, move the return inside the same condition and return '' otherwise.
-{% set dummy_query %} - -- This query is needed for Elementary to work correctly with dbt Fusion - select 'on-run-end' -{% endset %} -{{ return(dummy_query) }} +{% if execute and not elementary.is_docs_command() %} + {% set dummy_query %} + -- This query is needed for Elementary to work correctly with dbt Fusion + select 'on-run-end' + {% endset %} + {{ return(dummy_query) }} +{% else %} + {{ return('') }} +{% endif %}Additionally, the inner condition checks “not execute” even though the outer guard already ensures execute is true; that part is unreachable and can be dropped for clarity. Want a patch for that too?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.gitignore(3 hunks)macros/edr/system/hooks/on_run_end.sql(1 hunks)macros/edr/system/hooks/on_run_start.sql(1 hunks)models/run_results.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-15T19:31:54.689Z
Learnt from: haritamar
PR: elementary-data/dbt-data-reliability#825
File: models/run_results.yml:139-145
Timestamp: 2025-07-15T19:31:54.689Z
Learning: In dbt-fusion (dbt 2.0+), the `meta` configuration for models should be nested under the `config` block (i.e., `config: meta:`), not as a top-level `meta` key. This is different from traditional dbt where `meta` was a sibling of `config`.
Applied to files:
models/run_results.yml
🔇 Additional comments (4)
.gitignore (3)
15-15: Good addition: cover virtualenvs with .venv/This complements venv/ and avoids accidental commits of local envs.
29-29: Good addition: ignore dbt_internal_packages/Prevents committing dbt’s internal dependencies; aligns with dbt Fusion/dbt Core behaviors.
5-5: Typo? Use package-lock.json instead of package-lock.ymlNode’s lockfile is package-lock.json. If the intent is to ignore the Node lockfile, update accordingly.
-package-lock.yml +package-lock.json⛔ Skipped due to learnings
Learnt from: haritamar PR: elementary-data/dbt-data-reliability#825 File: package-lock.yml:1-5 Timestamp: 2025-07-15T19:28:12.728Z Learning: The correct filename for dbt's lockfile is `package-lock.yml`, not `packages.lock`. The `dbt deps` command generates and uses `package-lock.yml` to lock dependency versions.Learnt from: haritamar PR: elementary-data/dbt-data-reliability#825 File: package-lock.yml:1-5 Timestamp: 2025-07-15T19:28:12.728Z Learning: The correct filename for dbt's lockfile is `package-lock.yml`, not `packages.lock`. The `dbt deps` command generates and uses `package-lock.yml` to lock dependency versions. This is the official and documented filename as of dbt v1.7 and onward.models/run_results.yml (1)
139-144: All dbt_run_results entries use config.meta correctlyI searched for other instances of
name: dbt_run_resultsin models/run_results.yml and found only the single definition at lines 133–144, which already nestsdeprecated_columnsunderconfig.meta. No further changes are needed.
| {% if elementary.is_test_command() %} | ||
| {{ elementary.create_elementary_tests_schema() }} | ||
| {% endif %} | ||
|
|
||
| {{ return("select 'on-run-start'") }} | ||
| {% endmacro %} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Risk: final return likely discards earlier emitted SQL (tests schema creation)
Using {{ return("select 'on-run-start'") }} at the end can override any previously rendered SQL (e.g., {{ elementary.create_elementary_tests_schema() }}), so the schema creation may no longer run in test commands.
Refactor to collect statements and return them together to preserve behavior.
- {% if elementary.is_test_command() %}
- {{ elementary.create_elementary_tests_schema() }}
- {% endif %}
-
- {{ return("select 'on-run-start'") }}
+ {# Collect hook SQLs to ensure nothing is lost when returning #}
+ {% set hook_sqls = [] %}
+ {% if elementary.is_test_command() %}
+ {% set _create_sql %}{{ elementary.create_elementary_tests_schema() }}{% endset %}
+ {% do hook_sqls.append(_create_sql) %}
+ {% endif %}
+ {% do hook_sqls.append("select 'on-run-start'") %}
+ {{ return(hook_sqls | join(';\n')) }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {% if elementary.is_test_command() %} | |
| {{ elementary.create_elementary_tests_schema() }} | |
| {% endif %} | |
| {{ return("select 'on-run-start'") }} | |
| {% endmacro %} | |
| {# Collect hook SQLs to ensure nothing is lost when returning #} | |
| {% set hook_sqls = [] %} | |
| {% if elementary.is_test_command() %} | |
| {% set _create_sql %}{{ elementary.create_elementary_tests_schema() }}{% endset %} | |
| {% do hook_sqls.append(_create_sql) %} | |
| {% endif %} | |
| {% do hook_sqls.append("select 'on-run-start'") %} | |
| {{ return(hook_sqls | join(';\n')) }} | |
| {% endmacro %} |
🤖 Prompt for AI Agents
In macros/edr/system/hooks/on_run_start.sql around lines 22 to 27, the final {{
return("select 'on-run-start'") }} will overwrite any previously emitted SQL
(such as the elementary.create_elementary_tests_schema() call) causing the tests
schema creation to be discarded; change the macro to accumulate statements
(e.g., push the conditional schema-creation SQL into a list or a buffer
variable) and then return a single concatenated string that includes both the
schema creation SQL (when applicable) and the "select 'on-run-start'" statement
so earlier emitted SQL is preserved.
1 participant
To tackle this issue.
Note on the
on-run-start/endpart: I don't really like the solution. Trouble is that this method will fire a query at the database. In Core this was failing silently, but in Fusion you will get an explicit error. A better solution would be to run an operation on at the start/end, but I don't think we can do that.Summary by CodeRabbit