fivetran / dbt_linkedin

Fivetran's Linkedin Ads dbt package
https://fivetran.github.io/dbt_linkedin/
Apache License 2.0
2 stars 12 forks source link

[Bug] use dbt_utils.current_timestamp_in_utc() #18

Closed fivetran-jamie closed 1 year ago

fivetran-jamie commented 2 years ago

Is there an existing issue for this?

Describe the issue

We compare Linkedin timestamps (which are in UTC) to {{ dbt_utils.current_timestamp }} which pulls the current timestamp from the LOCAL warehouse. What we should be using instead is {{ dbt_utils.current_timestamp_in_utc }} which actually exists in the same .sql file as the normal current_timestamp macro in dbt_utils

from https://getdbt.slack.com/archives/C01D1R2JLLA/p1650539408651009

Relevant error log or model output

No response

Expected behavior

compare apples to apples

dbt Project configurations

na

Package versions

0.4.0

What database are you using dbt with?

snowflake

dbt Version

na

Additional Context

No response

Are you willing to open a PR to help address this issue?

fivetran-jamie commented 2 years ago

So this will actually become a non-issue soon. in every other ad reporting package, we only join in the latest version of entities like accounts or campaigns or creatives. We will be extending this to linkedin as well, as it currently joins in the historical version of the entity that was valid at the time.

So, these lines

 from metrics
    left join creatives
        on metrics.creative_id = creatives.creative_id
        and {{ fivetran_utils.timestamp_add('day', 1, 'metrics.date_day') }} >= creatives.valid_from
        and {{ fivetran_utils.timestamp_add('day', 1, 'metrics.date_day') }} <= coalesce(creatives.valid_to, {{ fivetran_utils.timestamp_add('day', 1, dbt_utils.current_timestamp()) }})

will instead look like

 from metrics
    left join creatives
        on metrics.creative_id = creatives.creative_id
        and creatives.is_lastest_version = true

So we won't be using current_timestamp at all anymore!

We'll be rolling this out in the next month or so as part of our big Ad Reporting V2 updates.

fivetran-joemarkiewicz commented 1 year ago

Closing this issue out as the date range in the join clause has been removed from the latest version of the package.