Feature/add union data #123

fivetran-jamie · 2024-01-08T15:57:17Z

PR Overview

This PR will address the following Issue/Feature:
fivetran/dbt_hubspot#130

This PR will result in the following new package version:

v0.15.0

Please detail what change(s) this PR introduces and any additional information that should be known during the review of this PR:
introduces support for running the package on a union of connectors

adds union_data macro to each _tmp model
includes source relation in each staging model
adds yml documentation for source_relation
includes source_relation in all uniqueness tests
documents unioning ability in README
ensures _dbt_source_relation does not slip in when hubspot__pass_through_all_columns is true
disables default source if unioning is occurring

dbt_hubspot_source v0.15.0

PR #123 includes the following updates:

🎉 Feature Update 🎉

This release supports running the package on multiple Hubspot sources at once! See the README for details on how to leverage this feature.

🛠️ Under the Hood 🛠️

Included auto-releaser GitHub Actions workflow to automate future releases.
Included Github Actions workflow to check for docs updates.
Updated the maintainer PR template to resemble the most up to date format.

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 (if applicable) -- ran with diff hubspot__pass_through_all_columns values and specific passthrough columns with add_property_label: true

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.

See Hex notebook validating transform models

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)
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) -- will do post-approval

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

🇨🇱

README.md

…ssed through

fivetran-joemarkiewicz

@fivetran-jamie the results of this PR look great! I just have a few questions and suggestions in the comments below that I would like your eyes on before we approve this. Let me know if you have any questions or want to chat about these further.

fivetran-joemarkiewicz · 2024-02-02T19:56:54Z

.github/workflows/check_docs.yml

+name: 'check docs'
+on:
+  push:
+    branches:
+      - main
+  pull_request:
+    branches:
+      - main
+
+jobs:
+  changed-files:
+    runs-on: ubuntu-latest
+    name: test changed-files
+    steps:
+      - uses: actions/checkout@v3
+        with:
+          fetch-depth: 0
+
+      - name: Get changed files
+        id: changed-files
+        uses: tj-actions/[email protected]
+        with:
+          files: docs/**
+
+      - name: Check to see if docs folder hasn't changed
+        if: steps.changed-files.outputs.any_changed == 'false'
+        run: |
+          echo "Docs have not been regenerated."
+          exit 1
+
+      - name: Check if docs folder has changed
+        if: steps.changed-files.outputs.any_changed == 'true'
+        run: |
+          echo "Docs have been regenerated!"
+          exit 0 


Let's actually remove this. We are planning on taking a new approach this year on docs checking and generation.

CHANGELOG.md

fivetran-joemarkiewicz · 2024-02-02T19:57:40Z

CHANGELOG.md

+
+## 🛠️ Under the Hood 🛠️
+- Included auto-releaser GitHub Actions workflow to automate future releases.
+- Included Github Actions workflow to check for docs updates.


After removing the file we should also remove this entry.

fivetran-joemarkiewicz · 2024-02-02T20:01:08Z

README.md

+To properly incorporate all of your Hubspot connectors into your project's DAG:
+1. Define each of your sources in a `.yml` file in your project. Utilize the following template to leverage our table and column documentation.
+
+  <details><summary><i>Expand for source configuration template</i></summary><p>


I like this idea, but I worry it is going to be a pain to maintain going forward. Is there any other way we can document this without explicitly showing the yml example like you have below?

fivetran-joemarkiewicz · 2024-02-02T21:25:59Z

models/stg_hubspot__company.sql

        {% if all_passthrough_column_check('stg_hubspot__company_tmp',get_company_columns()) > 0 %}
        -- just pass everything through if extra columns are present, but ensure required columns are present.
        ,{{ 
            fivetran_utils.remove_prefix_from_columns(
                columns=adapter.get_columns_in_relation(ref('stg_hubspot__company_tmp')), 
-                prefix='property_', exclude=get_macro_columns(get_company_columns()))
+                prefix='property_', exclude=(get_macro_columns(get_company_columns()) + ['_dbt_source_relation']))


Can you explain the need for this addition?

This comment applies to the other models with the similar code update.

yeah essentially we don't want to include _dbt_source_relation (which is created by union_data/union_relation) in the remove_prefix_from_columns macro call.

without adding it to the exclude list, users passing through all columns would end up with both a source_relation and _dbt_source_relation column, which is redundant and a lil confusing. thus, this change makes sure that these users just have the more-nicely-named source_relation field.

fivetran-joemarkiewicz · 2024-02-02T21:30:59Z

packages.yml

+# - package: fivetran/fivetran_utils
+#   version: [">=0.4.0", "<0.5.0"]
+- git: https://github.com/fivetran/dbt_fivetran_utils.git
+  revision: feature/enhance-union-data
+  warn-unpinned: false
+


Reminder to swap before release

fivetran-jamie added 3 commits January 2, 2024 15:40

running successfully

63ecab2

update readme

b668a58

docs and buildkite

e35e9eb

fivetran-jamie self-assigned this Jan 8, 2024

fivetran-jamie added 2 commits January 8, 2024 17:06

doc tweak

9caf6ad

add missing source_relation

93ca4f3

fivetran-jamie commented Jan 9, 2024

View reviewed changes

README.md Show resolved Hide resolved

fivetran-jamie added 3 commits January 9, 2024 12:19

doc tweaks

ea142f4

add source relation to add_property_labels macro joins

4e390b5

make sure _dbt_source_relation doesn't slip in when all fields are pa…

0644b82

…ssed through

fivetran-jamie marked this pull request as ready for review January 9, 2024 18:35

fivetran-jamie added 3 commits January 9, 2024 15:51

exclude _dbt_source_relation from all_passthrough_column_check macro

b6900a5

disable source dynamically

1f7cda8

add github stuff

c8c51da

fivetran-jamie requested a review from fivetran-joemarkiewicz January 10, 2024 19:11

polish readme

7b2a5ca

fivetran-joemarkiewicz requested changes Feb 2, 2024

View reviewed changes

fivetran-jamie added 4 commits February 5, 2024 11:25

joe feedback

7a37bb8

joe feedback

363527b

switch source defining instructions

ad0c752

tiny tweak to readme

7ef069f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/add union data #123

Feature/add union data #123

fivetran-jamie commented Jan 8, 2024 •

edited

Loading

fivetran-joemarkiewicz left a comment

fivetran-joemarkiewicz Feb 2, 2024

fivetran-jamie Feb 5, 2024

fivetran-joemarkiewicz Feb 2, 2024

fivetran-jamie Feb 5, 2024

fivetran-joemarkiewicz Feb 2, 2024

fivetran-jamie Feb 5, 2024

fivetran-joemarkiewicz Feb 2, 2024

fivetran-joemarkiewicz Feb 2, 2024

fivetran-jamie Feb 5, 2024

fivetran-joemarkiewicz Feb 2, 2024

Feature/add union data #123

Are you sure you want to change the base?

Feature/add union data #123

Conversation

fivetran-jamie commented Jan 8, 2024 • edited Loading

PR Overview

dbt_hubspot_source v0.15.0

🎉 Feature Update 🎉

🛠️ Under the Hood 🛠️

PR Checklist

Basic Validation

Detailed Validation

Standard Updates

dbt Docs

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

fivetran-joemarkiewicz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fivetran-jamie commented Jan 8, 2024 •

edited

Loading