search

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
142 stars 56 forks source link

update semantic model name #105

Closed Jstein77 closed 11 months ago

Jstein77 commented 11 months ago

Please provide your name and company dbt labs Link the issue/feature request which this PR is meant to address https://github.com/fivetran/dbt_ad_reporting/issues/104

Detail what changes this PR introduces and how this addresses the issue/feature request linked above. This PR renames the semantic model from ad_reporting__ad_report --> ad_report How did you validate the changes introduced within this PR? dbt parse runs as expected Which warehouse did you use to develop these changes? Snowflake Did you update the CHANGELOG?

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)

Provide an emoji that best describes your current mood

:dancer: **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](https://www.surveymonkey.com/r/DQ7K7WW) on our existing dbt packages or what you'd like to see next. **PR Template** - [Community Pull Request Template](?expand=1&template=pull_request_template.md) (default) - [Maintainer Pull Request Template](?expand=1&template=maintainer_pull_request_template.md) (to be used by maintainers)
fivetran-joemarkiewicz commented 11 months ago

@Jstein77 thanks for opening this PR! Out of curiosity, do you feel this would be a breaking change (resulting in v1.7.0) or just a patch upgrade (resulting in v1.6.1)? I am unsure how this impacts the semantic layer users and would want to account for this change appropriately.

Jstein77 commented 11 months ago

Chatted with @QMalcolm about this. I think this is a patch upgrade/bug fix. We didn't upgrade the version for dbt-semantic-interfaces with this release, and it's unlikely that anyone is referencing the semantic model name anywhere so this shouldn't break any workflows.

QMalcolm commented 11 months ago

Chatted with @QMalcolm about this. I think this is a patch upgrade/bug fix. We didn't upgrade the version for dbt-semantic-interfaces with this release, and it's unlikely that anyone is referencing the semantic model name anywhere so this shouldn't break any workflows.

To clarify we didn't bump the minor version for this in dbt-core or dbt-semantic-interfaces, we did it as a patch release. This is because it is a bug fix, it's just an odd bug fix in that the issue occurs in MetricFlow, but needs to be fixed in dbt-semantic-interfacesand dbt-core

fivetran-joemarkiewicz commented 11 months ago

@Jstein77 and @QMalcolm thanks for confirming! With this information I feel comfortable rolling this update out as a patch release. My team recognizes code freezes on Fridays, so I will make some minor updates to your branch (ie. docs regen, version bump, changelog entry) and then plan for this to be released early next week.

Jstein77 commented 11 months ago

@fivetran-joemarkiewicz Thank you!