feature-bug/created-date-and-utils-star

[fivetran-joemarkiewicz]

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 be created_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 🚨

• The created_at field within the stg_hubspot__contact model has been renamed to created_date to be consistent with the source data. Additionally, this ensure there are no duplicate column errors when passing through all property_* columns which could potentially conflict with property_created_at.

Feature Updates

• All stg_hubspot__*_tmp models have been updated to leverage the dbt_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:

• dbt compile
• dbt run –full-refresh
• dbt run
• dbt test
• dbt run –vars (vars used in the integration tests)

Before marking this PR as "ready for review" the following have been applied:

• The appropriate issue has been linked and tagged
• You are assigned to the corresponding issue and this PR
• BuildKite integration tests are passing

Detailed Validation

Please acknowledge that the following validation checks have been performed prior to marking this PR as "ready for review":

• You have validated these changes and assure this PR will address the respective Issue/Feature.
• You are reasonably confident these changes will not impact any other components of this package or any dependent packages.
• You have provided details below around the validation steps performed to gain confidence in these changes.

dbt_utils.star() validaton

• I was able to confirm that the inclusion of the 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.
• Original compiled code

select *
from "postgres"."hubspot_source_integration_tests_6"."company_property_history_data"

• New compiled code

select "company_id",
  "timestamp",
  "_fivetran_synced",
  "source",
  "source_id",
  "name",
  "value"
from "postgres"."hubspot_source_integration_tests_6"."company_property_history_data"

• I also want to add that I tried to leverage the var() in the star macro. However, this caused the following error:

18:58:41  Running with dbt=1.6.3
18:58:41  Registered adapter: postgres=1.6.3
18:58:42  Encountered an error:
Compilation Error in model stg_hubspot__company_property_history_tmp (models/tmp/stg_hubspot__company_property_history_tmp.sql)
  Macro star expected a Relation but received the value: "postgres"."hubspot_source_integration_tests_6"."stg_hubspot__company_property_history_tmp"
  
  > in macro _is_relation (macros/jinja_helpers/_is_relation.sql)
  > called by macro default__star (macros/sql/star.sql)
  > called by macro star (macros/sql/star.sql)
  > called by model stg_hubspot__company_property_history_tmp (models/tmp/stg_hubspot__company_property_history_tmp.sql)

• Therefore, we needed to leverage the 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 the var(). 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

• I was able to recreate the initial error the customer was seeing. Then once using the version of code that is applied in this PR I saw the issue be resolved. The 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:

• Package versioning has been appropriately indexed in the following locations:
  • indexed within dbt_project.yml
  • indexed within integration_tests/dbt_project.yml
• CHANGELOG has individual entries for each respective change in this PR
• README updates have been applied (if applicable)
• [n/a] DECISIONLOG updates have been updated (if applicable)
• Appropriate yml documentation has been added (if applicable)

dbt Docs

Please acknowledge that after the above were all completed the below were applied to your branch:

• docs were regenerated (unless this PR does not include any code or yml updates)

If you had to summarize this PR in an emoji, which would it be?

🌟

[fivetran-jamie]

looks and runs good! couple of doc-related comments

also for the created_at -> created_date part, is that something we should apply to other tables as well?

[fivetran-joemarkiewicz]

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.

[fivetran-jamie]

additions look great! good to merge into my branch