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/crm api v3 - contact merge audit removed #98

Merged
merged 17 commits into from
Mar 28, 2023

Conversation

fivetran-jamie
Copy link
Contributor

@fivetran-jamie fivetran-jamie commented Mar 21, 2023

Are you a current Fivetran customer?

internal

What change(s) does this PR introduce?

aligns with recent changes to the hubspot API

  • adds the new property_hs_calculated_merged_vids field to the contact model/macro and its seed data
  • explicitly casts all timestamp fields as timestamps
  • adds BigQuery-specific logic since the above connector change is blocked on BQ. Thus, they will have to use the old behavior, but we want to make it so that they can switch to the new way if they want (by setting hubspot_contact_merge_audit_enabled to FALSE.

Did you update the CHANGELOG?

  • Yes

Does this PR introduce a breaking change?

  • Yes (please provide breaking change details below.)
  • No (please provide an explanation as to how the change is non-breaking below.)

default behavior is different for most users (though they should end up with the same result...so maybe not?)

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)

  • Yes

Is this PR in response to a previously created Bug or Feature Request

How did you test the PR changes?

  • Buildkite
  • Local (please provide additional testing details below)

Select which warehouse(s) were used to test the PR

  • BigQuery
  • Redshift
  • Snowflake
  • Postgres
  • Databricks
  • Other (provide details below)

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.

@fivetran-jamie fivetran-jamie self-assigned this Mar 21, 2023
@fivetran-jamie
Copy link
Contributor Author

will regen docs before merging

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.

@fivetran-jamie just a few minor requests and callouts. Let me know if you have any other questions!

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated
[PR #98](https://github.com/fivetran/dbt_hubspot_source/pull/98) applies the following changes:
- Ensures that all timestamp fields are explicitly cast as timestamps (without timezones).
- Updates logic around the recently deprecated `CONTACT_MERGE_AUDIT` table.
- In [November 2022](https://fivetran.com/docs/applications/hubspot/changelog#november2022), the Fivetran Hubspot connector switched to v3 of the Hubspot CRM API, which deprecated the `CONTACT_MERGE_AUDIT` table and stored merged contacts in the `property_hs_calculated_merged_vids` column in the `CONTACT` table. **This has not been rolled out to BigQuery warehouses yet.**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way we can provide a little more direction on a timeline for BigQuery users? Or just provide some details to let BigQuery users to have an understanding of when this will be updated for them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bumped drew but he's ooo till monday

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
models/src_hubspot.yml Outdated Show resolved Hide resolved
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.

@fivetran-jamie this is looking great and I feel it is very close to being able to merge through. I do just have one remaining comment regarding the hubspot_contact_merge_audit_enabled variable and how we use it for BigQuery users. I think we need to rethink how we use this variable. We can chat during standup.

Additionally, I came across a bug when doing more rigorous testing and ended up opening PR #100 to address the issue. We can batch this with these changes. It would be great if you could review.

models/src_hubspot.yml Outdated Show resolved Hide resolved
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 working through the updates @fivetran-jamie! After final review, these look good to go!

@fivetran-jamie fivetran-jamie changed the base branch from main to release/v0.9.0 March 28, 2023 20:15
@fivetran-jamie fivetran-jamie merged commit 2e494ad into release/v0.9.0 Mar 28, 2023
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.

2 participants