Bug/add legacy support #27

fivetran-catfritz · 2024-10-17T23:19:26Z

PR Overview

This PR will address the following Issue/Feature:

This PR will result in the following new package version:

v0.7.0 due to schema changes

Please provide the finalized CHANGELOG entry which details the relevant changes included in this PR:

Features

For Fivetran Lever connectors created on or after July 27, 2024, the USER and INTERVIEWER_USER source tables have been renamed to USERS and INTERVIEW_USER, respectively. This package now prioritizes the USERS and INTERVIEW_USER tables if available, falling back to USER and INTERVIEWER_USER if not.

To continue using the USER and/or INTERVIEWER_USER tables, set the variables lever__using_users and/or lever__using_interview_user to false in your dbt_project.yml.

For more information, refer to the July 2024 connector release notes and the related README section.

Introduced the ability to union source data from multiple Lever connectors. For more details, see the related README section.

Bug fixes

Fixed an issue where the dbt package would error due to a missing CONTACT_LINK source table for users without source data, even though it was enabled in the Fivetran Connector. A null-filled table will now be generated in such cases.

Under the hood

Updated temporary models to union source data using the fivetran_utils.union_data macro.

Added the source_relation column in each staging model to identify the origin of each field, utilizing the fivetran_utils.source_relation macro.

Updated tests to include the new source_relation column.

PR Checklist

Basic Validation

Please acknowledge that you have successfully performed the following commands locally:

dbt run –full-refresh && dbt test
~~dbt run (if incremental models are present) && dbt test~~

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

The appropriate issue has been linked, tagged, and properly assigned
All necessary documentation and version upgrades have been applied
docs were regenerated (unless this PR does not include any code or yml updates)
BuildKite integration tests are passing
Detailed validation steps have been provided below

Detailed Validation

Please share any and all of your validation steps:

Similar updates as found in THIS Recharge PR.

Missing contact_link

Simulated CONTACT_LINK as a missing source, and the null table is properly created.

`user` vs `users` and `interviewer_user` vs `interview_user`

Include both versions as seeds to test both versions of each source. Snippet from run_models.sh

dbt run --target "$db" --full-refresh
dbt test --target "$db"
dbt run --vars '{lever__using_users: false, lever__using_interview_user: false, lever_using_posting_tag: false, lever_using_requisitions:false}' --target "$db" --full-refresh
dbt test --vars '{lever__using_users: false, lever__using_interview_user: false, lever_using_posting_tag: false, lever_using_requisitions:false}' --target "$db"

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

💃

fivetran-catfritz · 2024-10-21T23:37:28Z

dbt_project.yml

-  lever_using_posting_tag: true
-  lever_using_requisitions: true


These are true by default, so should not be included here.

fivetran-reneeli

Thanks @fivetran-catfritz ! Just had some comments

CHANGELOG.md

integration_tests/dbt_project.yml

Co-authored-by: Renee Li <[email protected]>

fivetran-catfritz

Thanks @fivetran-reneeli! I have updated.

integration_tests/dbt_project.yml

fivetran-reneeli

Just remembered one thing-- to uncollapse the dropdowns in the README. Other than that, looks good to go!

fivetran-joemarkiewicz

@fivetran-catfritz thanks for working through this PR! I have a few change requests and comments before this is ready to go. Let me know if you have any questions. Thanks!

README.md

CHANGELOG.md

fivetran-joemarkiewicz · 2024-10-24T19:47:00Z

models/stg_lever.yml

      - name: opportunity_id
        description: Unique ID of the opportunity.
-        tests:


Is there a reason the not_null test was removed for this field? The unique combo of columns doesn't test if the field is not null. We should probably add that back in.

Same comment for the other fields that had the not_null test removed.

Added these back.

fivetran-joemarkiewicz · 2024-10-24T20:27:59Z

models/tmp/stg_lever__interviewer_user_tmp.sql

+{{
+    fivetran_utils.union_data(
+        table_identifier='interviewer_user', 
+        database_variable='lever_database', 
+        schema_variable='interview_user' if var('lever__using_interview_user', lever_source.does_table_exist('interview_user')) else 'interviewer_user', 
+        default_database=target.database,
+        default_schema='lever',
+        default_variable='interview_user' if var('lever__using_interview_user', lever_source.does_table_exist('interview_user')) else 'interviewer_user',
+        union_schema_variable='lever_union_schemas',
+        union_database_variable='lever_union_databases'
+    )
+}}


For some reason this isn't working as I would expect when testing on two different schemas (one with interviewer_user and one with interview_user. It seems to work when interviewer_user is present, but not interview_user.

Strangely enough, the users vs user works without an issue. Would you be able to look into this and see what could be the cause or if I may be missing something.

Oh boy I finally figured out I pasted the switch logic into the wrong line... updated!

Co-authored-by: Joe Markiewicz <[email protected]>

fivetran-catfritz

@fivetran-joemarkiewicz Thanks for the review. Made the updates!

fivetran-catfritz · 2024-10-24T21:44:06Z

models/tmp/stg_lever__interviewer_user_tmp.sql

+{{
+    fivetran_utils.union_data(
+        table_identifier='interviewer_user', 
+        database_variable='lever_database', 
+        schema_variable='interview_user' if var('lever__using_interview_user', lever_source.does_table_exist('interview_user')) else 'interviewer_user', 
+        default_database=target.database,
+        default_schema='lever',
+        default_variable='interview_user' if var('lever__using_interview_user', lever_source.does_table_exist('interview_user')) else 'interviewer_user',
+        union_schema_variable='lever_union_schemas',
+        union_database_variable='lever_union_databases'
+    )
+}}


Oh boy I finally figured out I pasted the switch logic into the wrong line... updated!

CHANGELOG.md

README.md

fivetran-joemarkiewicz

LGTM

fivetran-catfritz

@fivetran-joemarkiewicz I added the null tests back!

fivetran-catfritz · 2024-10-26T05:01:31Z

models/stg_lever.yml

      - name: opportunity_id
        description: Unique ID of the opportunity.
-        tests:


Added these back.

fivetran-joemarkiewicz

Thanks @fivetran-catfritz! Approved with one last request to add an additional entry to the CHANGELOG for the test modification/updates.

fivetran-joemarkiewicz · 2024-10-28T14:30:59Z

CHANGELOG.md

+## Under the hood
+- Turned off freshness tests for `USER`/`USERS` and `INTERVIEW_USER`/`INTERVIEWER_USER` to avoid possible conflicts.
+- Updated temporary models to union source data using the `fivetran_utils.union_data` macro.
+- Added the `source_relation` column in each staging model to identify the origin of each field, utilizing the `fivetran_utils.source_relation` macro.


Some staging models had the not_null test added when it was previously being tested. Can we include a callout in here to mention this. No need to be explicit, but an blanket statement saying we made sure to add not_null tests for the relevant fields within each staging model.

fivetran-catfritz added 4 commits October 17, 2024 10:52

MagicBot/add-union-schema updates

14c7b86

magicbot revisions

411b208

update changelog

77748a8

update changelog and readme

9251921

fivetran-catfritz self-assigned this Oct 17, 2024

fivetran-catfritz added 3 commits October 17, 2024 19:53

update yml

6325ba2

update yml

e96214c

regen docs

273769c

fivetran-catfritz commented Oct 21, 2024

View reviewed changes

fivetran-reneeli reviewed Oct 22, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

integration_tests/dbt_project.yml Show resolved Hide resolved

fivetran-catfritz and others added 2 commits October 23, 2024 11:17

revie updates

f6ac89f

Update CHANGELOG.md

904057a

Co-authored-by: Renee Li <[email protected]>

fivetran-catfritz commented Oct 23, 2024

View reviewed changes

integration_tests/dbt_project.yml Show resolved Hide resolved

fivetran-reneeli approved these changes Oct 23, 2024

View reviewed changes

fivetran-catfritz added 2 commits October 24, 2024 12:54

update readme & yml

d0e31ed

regen docs

f06b3fb

fivetran-joemarkiewicz requested changes Oct 24, 2024

View reviewed changes

fivetran-catfritz and others added 3 commits October 24, 2024 20:07

update and regen docs

64f5244

Apply suggestions from code review

28f6d61

Co-authored-by: Joe Markiewicz <[email protected]>

update changelog

5c6c6b4

fivetran-catfritz commented Oct 25, 2024

View reviewed changes

fivetran-joemarkiewicz approved these changes Oct 25, 2024

View reviewed changes

fivetran-catfritz added 2 commits October 25, 2024 23:16

update test & regen docs

6ac2477

update test & regen docs

42745dc

fivetran-catfritz commented Oct 28, 2024

View reviewed changes

fivetran-joemarkiewicz approved these changes Oct 28, 2024

View reviewed changes

update changelog

4a3e0f9

fivetran-catfritz merged commit 72cbe9a into main Oct 28, 2024
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug/add legacy support #27

Bug/add legacy support #27

fivetran-catfritz commented Oct 17, 2024 •

edited

Loading

fivetran-catfritz Oct 21, 2024

fivetran-reneeli left a comment

fivetran-catfritz left a comment

fivetran-reneeli left a comment

fivetran-joemarkiewicz left a comment

fivetran-joemarkiewicz Oct 24, 2024

fivetran-joemarkiewicz Oct 24, 2024

fivetran-catfritz Oct 26, 2024

fivetran-joemarkiewicz Oct 24, 2024

fivetran-catfritz Oct 24, 2024

fivetran-catfritz left a comment

fivetran-catfritz Oct 24, 2024

fivetran-joemarkiewicz left a comment

fivetran-catfritz left a comment

fivetran-catfritz Oct 26, 2024

fivetran-joemarkiewicz left a comment

fivetran-joemarkiewicz Oct 28, 2024

fivetran-catfritz Oct 28, 2024

Bug/add legacy support #27

Bug/add legacy support #27

Conversation

fivetran-catfritz commented Oct 17, 2024 • edited Loading

PR Overview

Features

Bug fixes

Under the hood

PR Checklist

Basic Validation

Detailed Validation

Missing contact_link

user vs users and interviewer_user vs interview_user

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

Choose a reason for hiding this comment

fivetran-reneeli left a comment

Choose a reason for hiding this comment

fivetran-catfritz left a comment

Choose a reason for hiding this comment

fivetran-reneeli left a comment

Choose a reason for hiding this comment

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

fivetran-catfritz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fivetran-joemarkiewicz left a comment

Choose a reason for hiding this comment

fivetran-catfritz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

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

fivetran-catfritz commented Oct 17, 2024 •

edited

Loading

`user` vs `users` and `interviewer_user` vs `interview_user`