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

Use compiled_sql from manifest (no injected_sql, no run_results) #148

Merged
merged 4 commits into from
Oct 19, 2020

Conversation

jtcohen6
Copy link
Contributor

@jtcohen6 jtcohen6 commented Oct 15, 2020

resolves #146

Tested with dbt-labs/dbt-core#2834. Both ephemeral models and models that select from ephemeral models are looking good.

TODO before merge

  • We need to rebase dev/kiyoshi-kuromiya onto v0.18.1 to grab all the code around exposures. Then I'll need to regenerate the data/ artifacts.

Checklist

  • I have signed the CLA
  • I have generated docs locally, and this change appears to resolve the stated issue
  • I have updated the CHANGELOG.md and added information about my change to the "dbt next" section.

@cla-bot cla-bot bot added the cla:yes label Oct 15, 2020
@jtcohen6 jtcohen6 changed the title Replace injected_sql with compiled_sql Use compiled_sql from manifest (no injected_sql, no run_results) Oct 15, 2020
@jtcohen6 jtcohen6 requested a review from drewbanin October 19, 2020 14:26
Copy link
Contributor

@drewbanin drewbanin left a comment

Choose a reason for hiding this comment

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

This is awesome! Glad we can drop the dep on the run_results.json file... pretty funny we had been using it all this time :)

Approved on my end. Nice work @jtcohen6!

@jtcohen6 jtcohen6 merged commit 8422d4c into master Oct 19, 2020
@jtcohen6 jtcohen6 deleted the update/compiled-sql branch October 19, 2020 22:25
@jtcohen6 jtcohen6 mentioned this pull request Dec 17, 2020
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Account for v0.19 artifact updates
2 participants