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

update semantic model name #105

Merged

Conversation

Jstein77
Copy link
Contributor

@Jstein77 Jstein77 commented Oct 5, 2023

Please provide your name and company
dbt labs
Link the issue/feature request which this PR is meant to address
#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?

  • [x ] Yes

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)

  • [ x] Yes

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.

PR Template

@fivetran-joemarkiewicz
Copy link
Contributor

@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
Copy link
Contributor Author

Jstein77 commented Oct 5, 2023

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
Copy link

QMalcolm commented Oct 5, 2023

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-interfaces and dbt-core

@fivetran-joemarkiewicz
Copy link
Contributor

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

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.

Thanks for opening this PR @Jstein77 and for providing guidance on how to navigate the relationship of this package and the dbt semantic layer.

I made a few updates to your branch so it may be ready for merging to main. With that, this looks good on my end! I will kick off our internal release review process for this PR. However, please note that we enforce code freezes on Fridays. Therefore, this likely will wait to be released until early next week.

@Jstein77
Copy link
Contributor Author

Jstein77 commented Oct 6, 2023

@fivetran-joemarkiewicz Thank you!

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