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

Revamp for changes to Google Ads connector #25

Merged
merged 12 commits into from
Feb 23, 2022

Conversation

fivetran-joemarkiewicz
Copy link
Contributor

Are you a current Fivetran customer?

Fivetran created PR

What change(s) does this PR introduce?

This PR accounts for the changes to the Google Ads API connector from a few days ago. The major updates include:

  • Removing the ad_url_history table
  • Renaming the account table to account_history and treating it as a history table
  • Performing the utm logic using the final_urls field within the ad_history model.

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.)

This will be breaking change for existing users and we should highly encourage users to ensure they upgrade if they have not done so already.

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)

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 fivetran-joemarkiewicz marked this pull request as ready for review February 4, 2022 17:17
@fivetran-joemarkiewicz fivetran-joemarkiewicz self-assigned this Feb 4, 2022
@fivetran-sheringuyen
Copy link
Contributor

Ran a dbt test and got the below error, was this the error you mentioned we should be receiving? Other than that, looks good to me!

23:34:28 Warning in test dbt_expectations_expect_column_values_to_not_match_regex_list_stg_google_ads__ad_history_source_final_urls__any___ (models/google_ads_connector/stg_google_ads.yml) 23:34:28 Got 1 result, configured to warn if != 0 23:34:28 23:34:28 compiled SQL at target/compiled/google_ads_source/models/google_ads_connector/stg_google_ads.yml/dbt_expectations_expect_column_f02cb56d69a9df1e600b3958899ceaa5.sql 23:34:28 23:34:28 Done. PASS=0 WARN=1 ERROR=0 SKIP=0 TOTAL=1

@hendrikjd
Copy link

I'm a Fivetran user trying to install this package. I'm on the newer Google Ads API and have the package configured accordingly. Trying to run it for the first time and getting this error: Error in model stg_google_ads__ad_final_url_history_tmp ... ad_final_url_history was not found.

Looks to me like this PR will fix this issue. Just commenting in hopes that it can be merged soon!

@fivetran-joemarkiewicz
Copy link
Contributor Author

Ran a dbt test and got the below error, was this the error you mentioned we should be receiving? Other than that, looks good to me!

23:34:28 Warning in test dbt_expectations_expect_column_values_to_not_match_regex_list_stg_google_ads__ad_history_source_final_urls__any___ (models/google_ads_connector/stg_google_ads.yml) 23:34:28 Got 1 result, configured to warn if != 0 23:34:28 23:34:28 compiled SQL at target/compiled/google_ads_source/models/google_ads_connector/stg_google_ads.yml/dbt_expectations_expect_column_f02cb56d69a9df1e600b3958899ceaa5.sql 23:34:28 23:34:28 Done. PASS=0 WARN=1 ERROR=0 SKIP=0 TOTAL=1

Yup that is the warning I was looking for. Thanks so much for reviewing @fivetran-sheringuyen 😄

@fivetran-joemarkiewicz fivetran-joemarkiewicz merged commit 4ef2794 into main Feb 23, 2022
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