Feature/add action result #39

fivetran-catfritz · 2024-10-26T02:26:19Z

PR Overview

This PR will address the following Issue/Feature:

internal ticket

This PR will result in the following new package version:

v0.12.0 due to schema change

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

Breaking Changes:

The action_result field is now included in the following models, allowing users to filter records based on the action_result value:

No grain change:

stg_marketo__activity_send_email

marketo__email_sends_deduped

marketo__email_sends

Grain change (now includes action_result):

marketo__email_stats__by_email_template

marketo__email_stats__by_lead

marketo__email_templates

marketo__leads

Documentation updates:

Add missing definitions to dbt documents.

Under the Hood:

Added consistency integration tests for the models listed above.

Removed unnecessary tests from intermediate models to optimize resource usage.

Updated seed data with action_result

Feature

Added the action_result field to the stg_marketo__activity_send_email model to capture the result of email actions and enable filtering in downstream models.

Documentation update

Update dbt docs with action_result

Under the hood

Updated seed data with action_result

Updated macro marketo_activity_send_email_data with action_result

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:

consistency tests pass for email_sends when excluding the added action_result field.
It is expected to fail for email_templates and leads due to grain change.

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

💃

fivetran-joemarkiewicz

@fivetran-catfritz thanks for this PR. Please see my requests below in addition to removing the action_result field from the aggregate models like we discussed in our meeting earlier. Thanks!

CHANGELOG.md

fivetran-joemarkiewicz · 2024-10-30T20:46:20Z

packages.yml

+# - package: fivetran/marketo_source
+#   version: [">=0.12.0", "<0.13.0"]
+- git: https://github.com/fivetran/dbt_marketo_source.git
+  revision: feature/add-action-result
+  warn-unpinned: false


Reminder to swap before release

Reminder here again to swap before merge

fivetran-catfritz

@fivetran-joemarkiewicz As we discussed, I removed action_result from the models that would have a grain change and added a DECISIONLOG discussing this.

CHANGELOG.md

fivetran-joemarkiewicz

@fivetran-catfritz great job with these updates! I have a few small requests before this is ready for approval.

CHANGELOG.md

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

fivetran-catfritz

@fivetran-joemarkiewicz Updated and ready for review!

CHANGELOG.md

fivetran-joemarkiewicz

LGTM with just a few small comments. Also, would you be able to open a FR on this repo for us to modify the marketo__leads validation test. Currently we are testing to make sure every field matches between the prod and dev. However, this can be over 500 columns for some cases and it can lead to erroneous failures in the test and make it hard to decipher why it's failing. I imagine a straight forward lead row count would do the trick here and help with the massive width of the table.

fivetran-joemarkiewicz · 2024-11-01T18:33:08Z

packages.yml

+# - package: fivetran/marketo_source
+#   version: [">=0.12.0", "<0.13.0"]
+- git: https://github.com/fivetran/dbt_marketo_source.git
+  revision: feature/add-action-result
+  warn-unpinned: false


Reminder here again to swap before merge

fivetran-joemarkiewicz · 2024-11-01T18:38:38Z

integration_tests/dbt_project.yml

+    # enable when regenerating docs
+    marketo__enable_campaigns: true
+    marketo__enable_programs: true
+


I just noticed the models config is not included in here. Can we add that for easier future validation testing.

models: +schema: "marketo_{{ var('directed_schema','dev') }}"

…an/dbt_marketo into feature/add-action-result merge

fivetran-catfritz

@fivetran-joemarkiewicz I went ahead and updated the consistency_leads tests since it was straightforward. I gave it a test run too on our test data, and it is working!

fivetran-catfritz · 2024-11-01T20:19:57Z

integration_tests/dbt_project.yml

+    # enable when regenerating docs
+    marketo__enable_campaigns: true
+    marketo__enable_programs: true
+


fivetran-reneeli · 2024-11-26T19:15:29Z

CHANGELOG.md

+    - `marketo__email_sends_deduped`
+    - `marketo__email_sends`
+  - *Note:* If you have previously added this field via the `marketo__activity_send_email_passthrough_columns` variable, remove or alias it there to prevent duplicate column errors.
+- To minimize customer impact, we limited adding `action_result` to models where it does not alter the grain. Users needing `action_result` in aggregate models can derive it from the `marketo__email_sends` model using custom aggregation logic. For more details, refer to the [DECISIONLOG entry](https://github.com/fivetran/dbt_marketo/blob/main/CHANGELOG.md#action-result-field-introduction).


No change here, just a note I think the changelog in the PR template is flipped between the models under grain change vs not.

Ahh thank you!

fivetran-reneeli

Just some very light comments!

fivetran-reneeli · 2024-11-26T19:19:44Z

README.md

Do we need to update the description for marketo__email_sends in the README since that grain was impacted?

Actually I checked again, disregard that. The key itself didn't change

fivetran-reneeli

LGTM

fivetran-catfritz

Thanks @fivetran-reneeli made the updates!

fivetran-catfritz · 2024-11-26T20:55:33Z

CHANGELOG.md

+    - `marketo__email_sends_deduped`
+    - `marketo__email_sends`
+  - *Note:* If you have previously added this field via the `marketo__activity_send_email_passthrough_columns` variable, remove or alias it there to prevent duplicate column errors.
+- To minimize customer impact, we limited adding `action_result` to models where it does not alter the grain. Users needing `action_result` in aggregate models can derive it from the `marketo__email_sends` model using custom aggregation logic. For more details, refer to the [DECISIONLOG entry](https://github.com/fivetran/dbt_marketo/blob/main/CHANGELOG.md#action-result-field-introduction).


Ahh thank you!

fivetran-catfritz added 3 commits October 24, 2024 14:10

feature/add-action-result

07a1e68

updates

8e7cdbd

updates & regen docs

df8d33a

fivetran-catfritz self-assigned this Oct 26, 2024

fivetran-catfritz added 7 commits October 25, 2024 21:36

update seed & changelog

7fa3a2a

update seed

a1c7392

update yml

530f2a3

update tests

f35a7c4

update changelog

b90b44f

update readme

2112414

update changelog

eeefc6e

fivetran-catfritz requested a review from fivetran-joemarkiewicz October 28, 2024 14:58

fivetran-joemarkiewicz requested changes Oct 31, 2024

View reviewed changes

fivetran-catfritz added 2 commits October 31, 2024 17:04

no grain changes

1fbbb04

regen docs

6b4122b

fivetran-catfritz commented Oct 31, 2024

View reviewed changes

CHANGELOG.md Show resolved Hide resolved

CHANGELOG.md Outdated Show resolved Hide resolved

update yml

4583d91

fivetran-catfritz requested a review from fivetran-joemarkiewicz October 31, 2024 22:14

fivetran-joemarkiewicz requested changes Nov 1, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

CHANGELOG.md Show resolved Hide resolved

fivetran-catfritz and others added 2 commits November 1, 2024 11:46

add definitions & regen docs

e3925c4

Apply suggestions from code review

5023623

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

fivetran-catfritz commented Nov 1, 2024

View reviewed changes

CHANGELOG.md Show resolved Hide resolved

CHANGELOG.md Outdated Show resolved Hide resolved

fivetran-joemarkiewicz approved these changes Nov 1, 2024

View reviewed changes

fivetran-catfritz added 2 commits November 1, 2024 15:21

update test and yml

b08a547

Merge branch 'feature/add-action-result' of https://github.com/fivetr…

be7d0cf

…an/dbt_marketo into feature/add-action-result merge

fivetran-catfritz commented Nov 1, 2024

View reviewed changes

fivetran-reneeli self-requested a review November 26, 2024 17:24

fivetran-reneeli reviewed Nov 26, 2024

View reviewed changes

fivetran-reneeli approved these changes Nov 26, 2024

View reviewed changes

fivetran-catfritz added 2 commits November 26, 2024 14:56

Update CHANGELOG.md

71cbaff

Update packages.yml

b6f1621

fivetran-catfritz commented Nov 26, 2024

View reviewed changes

fivetran-catfritz merged commit 54928cf into main Nov 26, 2024
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/add action result #39

Feature/add action result #39

fivetran-catfritz commented Oct 26, 2024 •

edited

Loading

fivetran-joemarkiewicz left a comment

fivetran-joemarkiewicz Oct 30, 2024

fivetran-joemarkiewicz Nov 1, 2024

fivetran-catfritz left a comment

fivetran-joemarkiewicz left a comment

fivetran-catfritz left a comment

fivetran-joemarkiewicz left a comment

fivetran-joemarkiewicz Nov 1, 2024

fivetran-joemarkiewicz Nov 1, 2024

fivetran-catfritz Nov 1, 2024

fivetran-catfritz left a comment

fivetran-catfritz Nov 1, 2024

fivetran-reneeli Nov 26, 2024

fivetran-catfritz Nov 26, 2024

fivetran-reneeli left a comment

fivetran-reneeli Nov 26, 2024

fivetran-reneeli Nov 26, 2024

fivetran-reneeli left a comment

fivetran-catfritz left a comment

fivetran-catfritz Nov 26, 2024

Feature/add action result #39

Feature/add action result #39

Conversation

fivetran-catfritz commented Oct 26, 2024 • edited Loading

PR Overview

Breaking Changes:

Documentation updates:

Under the Hood:

Feature

Documentation update

Under the hood

PR Checklist

Basic Validation

Detailed Validation

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

fivetran-catfritz left a 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

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

fivetran-catfritz 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

fivetran-reneeli 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-reneeli 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-catfritz commented Oct 26, 2024 •

edited

Loading