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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

add reddit ads #83

Merged
merged 20 commits into from
Mar 20, 2023
Merged

add reddit ads #83

merged 20 commits into from
Mar 20, 2023

Conversation

fivetran-reneeli
Copy link
Contributor

Are you a current Fivetran customer?

Internal
What change(s) does this PR introduce?

Add in the reddit ads reports
Did you update the CHANGELOG?

  • Yes

Does this PR introduce a breaking change?

  • Yes (please provide breaking change details below.)
  • No (please provide an explanation as to how the change is non-breaking below.)

Did you update the dbt_project.yml files with the version upgrade (please leverage standard semantic versioning)? (In both your main project and integration_tests)

  • Yes

Is this PR in response to a previously created Bug or Feature Request

  • Yes, Issue/Feature [link bug/feature number here]
  • No

How did you test the PR changes?

  • Buildkite
  • Local (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.

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.

@fivetran-reneeli A few quick suggestions, but I am unable to have this run successfully in my local environment. See my errors below. I believe there are additional updates that need to be made to this package in order to integrate Reddit Ads successfully into the roll up reports.

image

Can you make the proper updates to ensure this will run. Once the updates are applied you can mark this for re-review.

CHANGELOG.md Outdated Show resolved Hide resolved
integration_tests/ci/sample.profiles.yml Outdated Show resolved Hide resolved
integration_tests/seeds/reddit_ads_post_report_data.csv Outdated Show resolved Hide resolved
packages.yml Outdated
Comment on lines 32 to 33
# - package: fivetran/reddit_ads
# version: [">=0.1.0", "<0.2.0"]
Copy link
Contributor

Choose a reason for hiding this comment

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

obligatory reminder to update before release.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

.buildkite/scripts/run_models.sh Outdated Show resolved Hide resolved
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.

@fivetran-reneeli this looks great! Thanks for working so efficiently to get this integrated into the package! We can now kick off the releases for the upstream packages.

Let's hold off to cut this release until reddit ads and reddit ads source are live on the hub. We can sync during standup tomorrow.

Also a last note, don't forget to bump the version of the package in the README in Step 2.

packages.yml Outdated
Comment on lines 32 to 34
- git: https://github.com/fivetran/dbt_reddit_ads.git
revision: main
warn-unpinned: false
Copy link
Contributor

Choose a reason for hiding this comment

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

Reminder to switch before release

@fivetran-reneeli
Copy link
Contributor Author

buildkite is failing on linked in versions, as expected

@fivetran-joemarkiewicz
Copy link
Contributor

@fivetran-reneeli can we switch back to the latest version of linkedin. I don't believe we will be able to cut the update before this release.

@fivetran-reneeli fivetran-reneeli merged commit c99a1f9 into main Mar 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:enhancement New functionality or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants