-
Notifications
You must be signed in to change notification settings - Fork 3
bug/date-spine-query #42
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
Conversation
| @@ -1,32 +1,36 @@ | |||
| {{ config(materialized='table') }} | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved this to dbt_project.yml
| int_apple_store__date_spine: | ||
| +materialized: table |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved here from the model config.
fivetran-joemarkiewicz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with one update suggestion and a FR creation request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're still referencing apple_store_source on line 11 of this file. We need to update this to apple_store to reflect the current state of the consolidated package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additionally, I noticed the identifier variables don't follow our standard naming convention:
source_name + `_` + source_table_name + `_identifier`.We should update the identifiers and the references to swap in the following way:
apple_store_crash_daily_identifier->apple_store_app_crash_daily_identifier
This doesn't need to be updated in this release, but please create a FR for us to update this in a future breaking change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Issue opened. #43
…t_apple_store into bug/date-spine-query merge
PR Overview
Package version introduced in this PR:
This PR addresses the following Issue/Feature(s):
Summary of changes:
int_apple_store__date_spineupdated the run query to use the stagingrefs instead of thesources.Submission Checklist
Changelog