Closed fivetran-joemarkiewicz closed 10 months ago
also for the created_at -> created_date part, is that something we should apply to other tables as well?
@fivetran-jamie great call out here! I hadn't thought about the need to update this in other places since the contact model was the only one the original issue was created within. However, when thinking about the issue as a whole I could see this happening within the company
, deal
, and ticket
models in addition to the contact
since we do a similar renaming. After reviewing them I felt it was best to take your advice and make the updates in this PR to account for those future possible issues. Please see my updates for the changes.
Additionally, when reviewing this slack thread I felt it would be an impactful and easy addition to the HubSpot package to introduce variables to disable the property history models. I introduced this variable for all but the ticket model since your PR is working heavily with this model. I would prefer we highlight that history model. Especially since tickets are disabled by default for now.
PR Overview
This PR will address the following Issue/Feature: Issue #117 and Feature #116
This PR will result in the following new package version:
v0.13.0
Since we are changing the
created_at
field to now becreated_date
, this will result in a breaking change that customers will need to take into account.Please detail what change(s) this PR introduces and any additional information that should be known during the review of this PR:
π¨ Breaking Changes π¨
created_at
field within thestg_hubspot__contact
model has been renamed tocreated_date
to be consistent with the source data. Additionally, this ensure there are no duplicate column errors when passing through allproperty_*
columns which could potentially conflict withproperty_created_at
.Feature Updates
stg_hubspot__*_tmp
models have been updated to leverage thedbt_utils.star()
macro. This ensures if the source dimension changes there is no potential for a mismatch in columns error that is commonly seen in Snowflake destinations.PR Checklist
Basic Validation
Please acknowledge that you have successfully performed the following commands locally:
Before marking this PR as "ready for review" the following have been applied:
Detailed Validation
Please acknowledge that the following validation checks have been performed prior to marking this PR as "ready for review":
dbt_utils.star() validaton
dbt_utils.star()
macro in each of the*_tmp
models did not cause unforeseen issues. They still operated as the normal*
had worked previously. However, now we see the full list of columns specified in the compiled code which will ensure the columns cannot mismatch when the source changes.I also want to add that I tried to leverage the
var()
in the star macro. However, this caused the following error:source()
call. I am comfortable with this is the use of our identifier variables allows the customer to still switch this around if they wish. I was considering adjusting the*_tmp
models to not use thevar()
. However, I didn't think this was entirely necessary and wanted to discuss with the team before applying those changes at scale in this update.Created At -> Created Date Validation
propery_created_at
field was added to the seed file so we may ensure this is being tested going forward.Standard Updates
Please acknowledge that your PR contains the following standard updates:
dbt Docs
Please acknowledge that after the above were all completed the below were applied to your branch:
If you had to summarize this PR in an emoji, which would it be?
π