Skip to content

Fix: Dependency handling when converting on-run-start / on-run-end hooks in dbt projects#4567

Merged
izeigerman merged 2 commits intomainfrom
fix-ref-source-in-on-run-start-end-dbt
May 28, 2025
Merged

Fix: Dependency handling when converting on-run-start / on-run-end hooks in dbt projects#4567
izeigerman merged 2 commits intomainfrom
fix-ref-source-in-on-run-start-end-dbt

Conversation

@izeigerman
Copy link
Contributor

Previously, the conversion process didn't handle refs and sources in on-run-start / on-run-end hooks. This fix addresses the issue by sourcing ref and source dependencies directly from the manifest, in addition to extracting them by statically analyzing the hook's Jinja code.

@izeigerman izeigerman requested a review from a team May 28, 2025 01:38
Copy link
Contributor

@themisvaltinos themisvaltinos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the change looks good, but I noticed an issue in the _load_environment_statements which is my mistake that environment_statements is overwritten on each iteration of the loop so it doesn’t accumulate EnvironmentStatements across multiple projects. I believe to fix this we should replace the assignment with:

environment_statements.extend(
    statements
    for _, statements in sorted(
        hooks_by_package_name.items(),
        key=lambda item: 0 if item[0] == context.project_name else 1,
    )
)

and properly indent it. But if not addressed here, I can handle this in a subsequent PR

@izeigerman
Copy link
Contributor Author

@themisvaltinos good catch. I've addressed your comment but keep in mind the following:

  1. It appears that the dbt loader only supports a single dbt project today
  2. Multiple dbt projects can have overlapping packages in which case we don't want the hooks to be duplicated. So I made hooks_by_package_name shared across projects instead.

@izeigerman izeigerman force-pushed the fix-ref-source-in-on-run-start-end-dbt branch from b25bc60 to 6163ef3 Compare May 28, 2025 16:10
@izeigerman izeigerman merged commit 2785fc9 into main May 28, 2025
23 checks passed
@izeigerman izeigerman deleted the fix-ref-source-in-on-run-start-end-dbt branch May 28, 2025 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants