Update api_calls to extract_summary #138

fivetran-catfritz · 2024-11-18T23:44:18Z

PR Overview

This PR will address the following Issue/Feature:

[Feature] Update package to use new extract_summary event instead of currently-being-deprecated api_call #137
[Documentation] Move badges below H1 heading #139

This PR will result in the following new package version:

v1.9.1 no schema change

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

Features

For Fivetran Platform Connectors created after November 2024, Fivetran has deprecated the api_call event in favor of extract_summary (release notes).

Accordingly, we have updated the fivetran_platform__connector_daily_events model to support the new extract_summary event while maintaining backward compatibility with the api_call event for connectors created before November 2024.

Under the Hood

Replaced the deprecated dbt.current_timestamp_backcompat() function with dbt.current_timestamp() to ensure all timestamps are captured in UTC.

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:

Updated seed data to make sure the values for extract_summary are pulled through.

dbt_fivetran_log/integration_tests/seeds/log.csv

Lines 33 to 34 in 89a3650

    
           D7UqnKYn6OT04HkUcPNjXA95qqp=,2024-11-18 16:26:29.719,2024-11-19 20:30:53.878,aft_gleeful,INFO,"{""total_queries"":15,""total_rows"":4810}",extract_summary,,456abc 
        
           h7UqnKYn6OT04HkUcPNjXA95qlp=,2024-11-19 16:26:29.719,2024-11-19 20:30:53.878,aft_gleeful,INFO,"{""total_queries"":5,""total_rows"":4810}",extract_summary,,456abc

Consistency tests pass

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

💃

fivetran-jamie

looks great to me! just minor requests but this is approved

fivetran-jamie · 2024-11-19T21:06:23Z

models/fivetran_platform__connector_daily_events.sql

@@ -63,7 +80,7 @@ connector_event_counts as (

 spine as (

-    {% if execute %}
+    {% if execute and flags.WHICH in ('run', 'build') %}


Could you note in the "under the hood" section of the CHANGELOG that this release supports dbt compile on a new schema before dbt run due to the above change?

Yes, updated!

fivetran-jamie · 2024-11-19T21:10:12Z

models/fivetran_platform__connector_daily_events.sql

@@ -78,7 +95,7 @@ spine as (
        {{ fivetran_utils.fivetran_date_spine(
            datepart = "day", 
            start_date =  "cast('" ~ first_date[0:10] ~ "' as date)", 
-            end_date = dbt.dateadd("week", 1, dbt.date_trunc('day', dbt.current_timestamp_backcompat() if target.type != 'sqlserver' else dbt.current_timestamp())) 
+            end_date = dbt.dateadd("week", 1, dbt.date_trunc('day', dbt.current_timestamp())) 


super duper minor -- would you mind updating the parallel line in the row_count__connector_daily_events test?

Good call. updated!

fivetran-catfritz

Thanks @fivetran-jamie made the updates!

fivetran-catfritz · 2024-11-20T18:26:38Z

models/fivetran_platform__connector_daily_events.sql

@@ -78,7 +95,7 @@ spine as (
        {{ fivetran_utils.fivetran_date_spine(
            datepart = "day", 
            start_date =  "cast('" ~ first_date[0:10] ~ "' as date)", 
-            end_date = dbt.dateadd("week", 1, dbt.date_trunc('day', dbt.current_timestamp_backcompat() if target.type != 'sqlserver' else dbt.current_timestamp())) 
+            end_date = dbt.dateadd("week", 1, dbt.date_trunc('day', dbt.current_timestamp())) 


Good call. updated!

fivetran-catfritz · 2024-11-20T18:34:48Z

models/fivetran_platform__connector_daily_events.sql

@@ -63,7 +80,7 @@ connector_event_counts as (

 spine as (

-    {% if execute %}
+    {% if execute and flags.WHICH in ('run', 'build') %}


Yes, updated!

fivetran-joemarkiewicz

LGTM with one small question/request around a comment from a previous update and if we should change it.

fivetran-joemarkiewicz · 2024-11-21T15:13:03Z

README.md

+# Fivetran Platform dbt Package ([Docs](https://fivetran.github.io/dbt_fivetran_log/))
+
+<p align="left">


Thanks for making this change as well!

fivetran-joemarkiewicz · 2024-11-21T15:36:48Z

models/fivetran_platform__connector_daily_events.sql

+    where event_subtype in (
+        'api_call', 'extract_summary', 'records_modified', 'create_table', 'alter_table',
+        'create_schema', 'change_schema_config') -- all schema changes


Small note that I know isn't related to your changes directly, but is it accurate to have the comment all schema changes in this filter? For example, api_call and extract_summary are not inherently schema changes and we make this clear in the below case when statement where we categorize perform the following:

when event_subtype in ('create_table', 'alter_table', 'create_schema', 'change_schema_config') then 'schema_change'

If this is true, can we change this comment to be all relevant event subtypes or something along those lines.

Makes sense--updated!

fivetran-catfritz

Updated.

fivetran-catfritz added 3 commits November 18, 2024 16:47

updates

296088f

update changelog

8e1b1ab

update changelog

a37121f

fivetran-catfritz self-assigned this Nov 18, 2024

fivetran-catfritz added 7 commits November 18, 2024 18:03

merge with main

90c8ec3

update changelog

e58eca0

update case when

4bbc894

update case when

7882b7b

update seed

c95351e

update seed

3d54ef3

regen docs

89a3650

fivetran-catfritz requested a review from fivetran-jamie November 19, 2024 19:39

fivetran-jamie approved these changes Nov 20, 2024

View reviewed changes

fivetran-catfritz added 3 commits November 20, 2024 12:23

update test

0bfed66

update changelog

3367542

update readme

65483eb

fivetran-catfritz commented Nov 20, 2024

View reviewed changes

This was linked to issues Nov 21, 2024

[Documentation] Move badges below H1 heading #139

Closed

[Feature] Update package to use new extract_summary event instead of currently-being-deprecated api_call #137

Closed

fivetran-joemarkiewicz self-requested a review November 21, 2024 15:11

fivetran-joemarkiewicz approved these changes Nov 21, 2024

View reviewed changes

update comment

eb451f6

fivetran-catfritz commented Nov 21, 2024

View reviewed changes

fivetran-catfritz merged commit 244f2c5 into main Nov 21, 2024
10 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update api_calls to extract_summary #138

Update api_calls to extract_summary #138

fivetran-catfritz commented Nov 18, 2024 •

edited

Loading

fivetran-jamie left a comment

fivetran-jamie Nov 19, 2024

fivetran-catfritz Nov 20, 2024

fivetran-jamie Nov 19, 2024

fivetran-catfritz Nov 20, 2024

fivetran-catfritz left a comment

fivetran-catfritz Nov 20, 2024

fivetran-catfritz Nov 20, 2024

fivetran-joemarkiewicz left a comment

fivetran-joemarkiewicz Nov 21, 2024

fivetran-joemarkiewicz Nov 21, 2024

fivetran-catfritz Nov 21, 2024

fivetran-catfritz left a comment

	D7UqnKYn6OT04HkUcPNjXA95qqp=,2024-11-18 16:26:29.719,2024-11-19 20:30:53.878,aft_gleeful,INFO,"{""total_queries"":15,""total_rows"":4810}",extract_summary,,456abc
	h7UqnKYn6OT04HkUcPNjXA95qlp=,2024-11-19 16:26:29.719,2024-11-19 20:30:53.878,aft_gleeful,INFO,"{""total_queries"":5,""total_rows"":4810}",extract_summary,,456abc

		# Fivetran Platform dbt Package ([Docs](https://fivetran.github.io/dbt_fivetran_log/))

		<p align="left">

Update api_calls to extract_summary #138

Update api_calls to extract_summary #138

Conversation

fivetran-catfritz commented Nov 18, 2024 • edited Loading

PR Overview

Features

Under the Hood

PR Checklist

Basic Validation

Detailed Validation

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

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

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

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

fivetran-catfritz commented Nov 18, 2024 •

edited

Loading