fivetran / dbt_ad_reporting

Fivetran's ad reporting dbt package. Combine your Facebook, Google, Pinterest, LinkedIn, Twitter, Snapchat, Microsoft, TikTok, Reddit, Amazon, and Apple Search advertising metrics using this package.
https://fivetran.github.io/dbt_ad_reporting/#!/overview
Apache License 2.0
146 stars 55 forks source link

[Bug] Incorrect documentation for LinkedIn Ads #58

Closed clay-walker closed 2 years ago

clay-walker commented 2 years ago

Is there an existing issue for this?

Describe the issue

The documentation listed here indicates that the user must include the "linkedin_schema" and "linkedin_database" vars in dbt_project.dbt.

The correct vars are:

linkedin_ads_schema linkedin_ads_database

Relevant error log or model output

No response

Expected behavior

Expected the ad models to build successfully

dbt Project configurations

vars: prod_database: analytics

ad_reportingapple_search_ads_enabled: False # by default this is assumed to be True ad_reportingpinterest_ads_enabled: False # by default this is assumed to be True ad_reportingtwitter_ads_enabled: False # by default this is assumed to be True ad_reporting__facebook_ads_enabled: False # by default this is assumed to be True ad_reportingsnapchat_ads_enabled: False # by default this is assumed to be True ad_reporting__tiktok_ads_enabled: False # by default this is assumed to be True

linkedin_schema: linkedin_ads linkedin_database: source_raw

google_ads_schema: google_ads google_ads_database: source_raw

microsoft_ads_schema: bingads microsoft_ads_database: source_raw

Package versions

packages:

What database are you using dbt with?

snowflake

dbt Version

1.0.1

Additional Context

No response

Are you willing to open a PR to help address this issue?

fivetran-joemarkiewicz commented 2 years ago

Hi @clay-walker thanks so much for opening this issue!

You are correct that the documentation for Linkedin ads schema and database variables is inaccurate and something we should update. I noticed you mention you are open to contributing and creating a PR to address this issue. If you are still open to this, we would be happy to review your PR and incorporate your contribution in our next release!

Let me know if you have any questions on the PR process. 🙏

fivetran-joemarkiewicz commented 2 years ago

Hi @clay-walker we are planning to release a new patch version of ad_reporting v1.0.1 tomorrow. If you would like your contribution to be included I would recommend opening a PR prior to tomorrow afternoon.

Otherwise, we can fold in the change along with our PR #57. Let me know if you have any questions 😄

clay-walker commented 2 years ago

Hey @fivetran-joemarkiewicz I'd love to contribute. Any guidelines I should be aware of before submitting a PR?

clay-walker commented 2 years ago

I cloned the repo locally, created a branch and staged my changes. Getting a 403 forbidden when I try to push to the repo. 🙃

fivetran-joemarkiewicz commented 2 years ago

Hey @clay-walker you will need to fork the repo in order to open a PR against it. I would recommend checking out this discourse post on how to best contribute. Thanks!

fivetran-joemarkiewicz commented 2 years ago

This has been addressed within the latest release of the package.