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

Fix: Rename folders to achieve correct materialization #19

Conversation

simon-stepper
Copy link
Contributor

Are you a current Fivetran customer?

Simon Stepper, BI Lead, Capdesk

What change(s) does this PR introduce?

At some point in dev history, the folders for the google_ads and adwords models were renamed. This change wasn't taken into account in the dbt_project.yml. Hence, tmp models are materialized as tables in the warehouse. As these are just select * models, this uses unnecessary storage for Fivetran customers. This PR adjusts the folder names in project.yml to execute the wished materialization.

Does this PR introduce a breaking change?

  • Yes (please provide breaking change details below.)
  • No (please provide explanation how the change is non breaking below.)
    We are only changing the materialization of the tmp models. This is done in the next dbt run and doesn't affect any downstream models.

Is this PR in response to a previously created Issue

  • Yes, Issue [link issue number here]
  • No

How did you test the PR changes?

  • CircleCi
  • Other (please provide additional testing details below)
  • Tested locally in my dev Snowflake environment. Tables were changed to views, downstream models still look the same.

Select which warehouse(s) were used to test the PR

  • BigQuery
  • Redshift
  • Snowflake
  • Postgres
  • Databricks
  • Other (provide details below)

Provide an emoji that best describes your current mood

😐

Feedback

We are so excited you decided to contribute to the Fivetran community dbt package! We continue to work to improve the packages and would greatly appreciate your feedback on our existing dbt packages or what you'd like to see next.

@fivetran-joemarkiewicz
Copy link
Contributor

Hi @NoToWarAlways thank you so much for opening this PR and catching this name change reflection in the dbt_project.yml! These changes make complete sense and I don't see how we missed this originally. Thanks for helping out with this PR!

There are a few minor updates I would like to make (upgrade the version number and add a CHANGELOG) before merging. Therefore, I will approve of this PR, but merge it with a staging branch to make the mentioned updates and then you can expect a new release of the package cut shortly after. Thanks!

@fivetran-joemarkiewicz fivetran-joemarkiewicz changed the base branch from main to bugfix/notowarealways-materialization-change November 2, 2021 13:20
Copy link
Contributor

@fivetran-joemarkiewicz fivetran-joemarkiewicz left a comment

Choose a reason for hiding this comment

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

Looks great, thanks again for opening this PR!

@fivetran-joemarkiewicz fivetran-joemarkiewicz merged commit cd0b745 into fivetran:bugfix/notowarealways-materialization-change Nov 2, 2021
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