Union capability - #52
Conversation
fivetran-catfritz
left a comment
fivetran-catfritz
left a comment
There was a problem hiding this comment.
It's looking in order! Left some suggestions and comments.
| ) }} | ||
|
|
||
| {% set columns_to_exclude = ['activity_id'] %} | ||
| {% set columns_to_exclude = ['activity_id', 'source_relation', 'email_template_id', 'email_send_id'] %} |
There was a problem hiding this comment.
I suggest something like the below, where source_relation would be passed to a var like consistency_test_exclude_columns. After this release, we would want to include source_relation in the test. This would apply to all the consistency tests.
| {% set columns_to_exclude = ['activity_id', 'source_relation', 'email_template_id', 'email_send_id'] %} | |
| {% set columns_to_exclude = ['activity_id', 'email_template_id', 'email_send_id'] + var('consistency_test_exclude_columns', [] %} |
| {% if execute %} | ||
| {% set first_date_query %} | ||
| select min( created_at ) as min_date from {{ source('marketo','lead') }} | ||
| select min( created_timestamp ) as min_date from {{ ref('stg_marketo__lead') }} |
There was a problem hiding this comment.
From our discussion, suggest to use _tmp so it's more 1:1
| select min( created_timestamp ) as min_date from {{ ref('stg_marketo__lead') }} | |
| select min(created_at) as min_date from {{ ref('stg_marketo__lead_tmp') }} |
| ) }} | ||
|
|
||
| {% set columns_to_exclude = ['activity_id'] %} | ||
| {% set columns_to_exclude = var('consistency_test_exclude_columns', []) %} |
There was a problem hiding this comment.
I think activity_id would always be excluded?
| {% set columns_to_exclude = var('consistency_test_exclude_columns', []) %} | |
| {% set columns_to_exclude = ['activity_id'] + var('consistency_test_exclude_columns', []) %} |
There was a problem hiding this comment.
Updating directly
There was a problem hiding this comment.
Hmm the order of operations is still off I think. Line 12 would overwrite the work of line 9? I tink you just need to replace line 6 with line 12, and remove line 12.
There was a problem hiding this comment.
Doh yeah, updating!
| enabled=var('fivetran_validation_tests_enabled', false) and var('marketo__enable_campaigns', true) and var('marketo__enable_programs', true) | ||
| ) }} | ||
|
|
||
| {% set exclude_cols = ['source_relation'] %} |
There was a problem hiding this comment.
I don't think we need this anymore.
| {% set exclude_cols = ['source_relation'] %} |
There was a problem hiding this comment.
Removing directly
| ) }} | ||
|
|
||
| {% set columns_to_exclude = ['activity_id'] %} | ||
| {% set columns_to_exclude = var('consistency_test_exclude_columns', []) %} |
There was a problem hiding this comment.
Hmm the order of operations is still off I think. Line 12 would overwrite the work of line 9? I tink you just need to replace line 6 with line 12, and remove line 12.
| else 1 end) over ( | ||
| partition by unioned.lead_id | ||
| order by coalesce(unioned.date_day, current_date) desc | ||
| partition by unioned.lead_id {{ partition_by_source_relation(alias='unioned') }} |
There was a problem hiding this comment.
I would do find and replace to add this wherever it's used.
| partition by unioned.lead_id {{ partition_by_source_relation(alias='unioned') }} | |
| partition by unioned.lead_id {{ marketo.partition_by_source_relation(alias='unioned') }} |
There was a problem hiding this comment.
Hm I already did -- I think this is outdated
fivetran-jamie
added
docs:ready
2 participants
PR Overview
Package version introduced in this PR:
This PR addresses the following Issue/Feature(s):
Summary of changes:
Submission Checklist
Consistency tests added for all end models and passing
Changelog