Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deploy dbt model dependencies before building daily models #467

Conversation

jeancochrane
Copy link
Contributor

@jeancochrane jeancochrane commented May 21, 2024

As discussed in https://github.com/ccao-data/aws-infrastructure/pull/26#discussion_r1609011282, there's currently an edge case in our dbt model dependency deployment pipeline that could bite us in the future. The problem is the combination of these three aspects of the pipeline:

  • Model dependencies are only deployed as part of the build-and-test-dbt workflow, which runs on PRs being merged to the main branch
  • Model dependencies are configured to expire every 90 days
  • Model dependencies are required by the build-daily-dbt-models workflow, which is scheduled to run once a day

As such, it's possible for build-daily-dbt-models to run without model dependencies existing, if they happen to have expired before it runs.

This PR fixes this edge case by updating build-daily-dbt-models to deploy model dependencies in the same way that build-and-test-dbt does.

I tested this PR by manually dispatching build-daily-dbt-models from this branch. (I ran the action just long enough to confirm that the deployment step succeeds, but cancelled it at that point, so the workflow appears to have failed.) Here's evidence that the deployment step succeeds: https://github.com/ccao-data/data-architecture/actions/runs/9193740823/job/25286017600#step:5:28

@jeancochrane jeancochrane marked this pull request as ready for review May 22, 2024 15:17
@jeancochrane jeancochrane requested a review from a team as a code owner May 22, 2024 15:17
@jeancochrane jeancochrane requested a review from dfsnow May 22, 2024 15:17
Copy link
Member

@dfsnow dfsnow left a comment

Choose a reason for hiding this comment

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

Nice, thanks for catching this!

@jeancochrane jeancochrane merged commit 6efd15a into master May 22, 2024
7 of 8 checks passed
@jeancochrane jeancochrane deleted the jeancochrane/deploy-dbt-model-dependencies-before-building-daily-models branch May 22, 2024 15:28
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.

2 participants