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

Service hub updates #30

Merged
merged 16 commits into from
Apr 1, 2021
Merged

Service hub updates #30

merged 16 commits into from
Apr 1, 2021

Conversation

fivetran-joemarkiewicz
Copy link
Contributor

@fivetran-joemarkiewicz fivetran-joemarkiewicz commented Mar 18, 2021

This PR includes the following changes:

  • HubSpot Service Hub model additions
    - Please note this PR currently is missing the pipelines and pipelines_stages models. A request is currently out for the Fivetran connector to include these tables. Once the tables are included at the connector level we will incorporate them within the PR
  • Custom schema update
  • Address the contact_id bug called out within PR Maintain support for dbt 0.17.2 #9
  • Service Hub README and .yml updates
  • Package upgrade to v0.3.0

@@ -97,3 +108,9 @@ seeds:
engagement_note_data:
+column_types:
engagement_id: "{{ 'int64' if target.name == 'bigquery' else 'bigint' }}"
ticket_company_data:
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need to add these if we're not configuring any column types or anything?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We most likely don't. They were carried over in case I needed to do any column config for seed data that I never ended up using. I can remove!

@@ -10,7 +10,7 @@ with base as (

select
id as contact_id,
{{ fivetran_utils.remove_prefix_from_columns(columns=columns, prefix='property_', exclude=['id']) }}
{{ fivetran_utils.remove_prefix_from_columns(columns=columns, prefix='property_', exclude=['id', 'contact_id']) }}
Copy link
Contributor

Choose a reason for hiding this comment

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

is contact_id supposed to be property_contact_id? based on https://github.com/fivetran/dbt_hubspot_source/pull/9/files

@fivetran-joemarkiewicz fivetran-joemarkiewicz marked this pull request as ready for review March 31, 2021 22:29
@fivetran-joemarkiewicz
Copy link
Contributor Author

This PR also incorporates the ideas provided by @jamesrayoub within PR #35 to allow users to add additional configuration to pass through columns and dynamically handle them within the staging models via newly added Fivetran utils.

These new fivetran utils are currently in an open PR and this should not be merged until the fivetran utils is merged and dependency is switched in this package.

packages.yml Outdated
@@ -3,4 +3,5 @@ packages:
version: [">=0.6.0", "<0.7.0"]

- git: "https://github.com/fivetran/dbt_fivetran_utils.git"
revision: add-passthrough-macro
Copy link
Contributor

Choose a reason for hiding this comment

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

update after merging

Copy link
Contributor

@fivetran-jamie fivetran-jamie left a comment

Choose a reason for hiding this comment

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

this looks good to me! we'll just need to make it very clear in the release that the deal/contact/company models now only bring a few columns by default, and that users will very likely/almost certainly have to use passthrough columns (and that they'll be a ton more flexible/useful than before)

@fivetran-joemarkiewicz fivetran-joemarkiewicz merged commit c467299 into master Apr 1, 2021
@fivetran-joemarkiewicz fivetran-joemarkiewicz deleted the service-hub-updates branch April 1, 2021 20:46
This was referenced Apr 1, 2021
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