[BUG] collect_freshness macro override throws warning in dbt source snapshot-freshness command

[aleix-cd]

When running the dbt source snapshot-freshness command, we are getting the following warning:

[WARNING]: Deprecated functionality
The 'collect_freshness' macro signature has changed to return the full query
result, rather than just a table of values. See the v1.5 migration guide for
details on how to update your custom macro:
https://docs.getdbt.com/guides/migration/versions/upgrading-to-v1.5

I believe this is due to the fact that the fivetran_utils package overrides the collect_freshness dbt macro (see this dbt Slack thread as reference: https://getdbt.slack.com/archives/C03SAHKKG2Z/p1683127164993329), and that it is expecting this
{{ return(load_result('collect_freshness')) }}
instead of this
{{ return(load_result('collect_freshness').table) }}

This is not blocking us, but it is not very pleasant to get warnings in each dbt source snapshot-freshness command since it creates undesired clutter.

Am I right in assuming this is the cause of this warning? If so, are there any plans to update the macro in fivetran_utils?

I actually recreated the macro in our repo, got rid of the .table part, and no warnings were raised when I ran the dbt source snapshot-freshness command.

My current dbt versions:

• dbt-core==1.6.5
• dbt-snowflake==1.6.4

[fivetran-joemarkiewicz]

Hi @aleix-cd thanks for opening this issue. You are correct that this is the cause of the warning. I had opened a branch with this fix, but then ended up not merging it as I thought a later version of dbt-core would resolve the error. It seems that did in case happen, but a warning was added in its place.

I think it is time now with this warning to consider rolling out the update for this macro. However, before pushing that change I want to consider if the macro override that we have in Fivetran Utils is even necessary anymore. I believe we added this macro to properly disable sources. However, that has become a feature in dbt for quite a while.

I will investigate this in the coming week and share my findings here. Regardless, we will plan to release a new version of fivetran_utils that either makes the suggested update or removes the override entirely.

[fivetran-avinash]

Hi @aleix-cd , thanks for taking the time to look into this issue! We did some further investigation and will be following @fivetran-joemarkiewicz 's plan to update the macro in the coming sprint.

The collect freshness macro was originally used to override dbt’s default collect_freshness macro which is called when running dbt source snapshot-freshness. This macro allows you to incorporate model enabling/disabling of variables into freshness tests. However dbt's new versions have essentially allowed for the same thing, so it is time to add new logic based on the version. We will be adding logic so that it defaults to dbt's macro logic based on the version of dbt you'll be using.

Although dbt's macro does make our collect_freshness macro redundant, our macro is being used across multiple Fivetran dbt packages, so we cannot remove it outright at this time. We will be filing feature requests across all our packages to change the logic to match dbt's config.

[fivetran-catfritz]

Hi @aleix-cd Which of our packages are you using? I have made a test branch for fivetran-utils that should fix the issue, and I'd like to make some test branches in those packages for you to try out on your end.

[aleix-cd]

Hi @aleix-cd Which of our packages are you using? I have made a test branch for fivetran-utils that should fix the issue, and I'd like to make some test branches in those packages for you to try out on your end.

Hi there @fivetran-catfritz ! We're using fivetran/ad_reporting, fivetran/fivetran_log and fivetran/zendesk 🙌 Happy to test this out :)

[fivetran-catfritz]

Thanks so much @aleix-cd! I've made some test branches for fivetran_log and zendesk, though I didn't make one for ad_reporting since it would be a bit involved. You can use the below for your packages.yml to test out these branches. Let me know how it goes!

packages:
  # - package: fivetran/ad_reporting
  #   version: [">=1.7.0", "<1.8.0"]

  - git: https://github.com/fivetran/dbt_fivetran_log.git
    revision: test/collect-freshness
    warn-unpinned: false
  - git: https://github.com/fivetran/dbt_zendesk.git
    revision: test/collect-freshness
    warn-unpinned: false

[aleix-cd]

@fivetran-catfritz any chance you can also apply changes in that branch for ad_reporting? We have a few custom models that rely on that package within our repo, and would make it much easier for us to test things out. Let me know!

[fivetran-joemarkiewicz]

@aleix-cd I actually may have found an easier way for you to test these changes (full disclosure I am not sure if this will work in dbt cloud but it does locally via dbt core).

Can you go your normal route of installing all your packages via dbt deps. Then after all your packages are installed, go into your packages.yml folder and comment out all your packages that you just installed and make add the following package dependency and ensure it is the only one not commented out.

packages:
  # - package: fivetran/zendesk
  #   version: [">=0.12.0", "<0.13.0"]

  # - package: fivetran/ad_reporting
  #   version: [">=1.7.0", "<1.8.0"]

  - git: https://github.com/fivetran/dbt_fivetran_utils.git
    revision: review/collect-freshness-1.4.0-test
    warn-unpinned: false 

Once you have your packages.yml like this, you will save the file and then rerun dbt deps (Note: you will not run dbt clean). Once that is successfully you should see the version of this package branch in your dbt_packages! You can then run dbt source freshness and any other commands you like to ensure all looks good on your end.

Let me know if you are able to try this and if it looks good. We are still planning to release by tomorrow or Wednesday, but it would greatly help if you can validate these changes. Thanks 😄

[aleix-cd]

Thanks for the workaround, @fivetran-joemarkiewicz ! It did work locally with dbt-core :)

Followed your steps, ran dbt source freshness, and no warnings were raised by the command! 💯

[fivetran-catfritz]

@aleix-cd Thanks for testing this out! This update is scheduled to be released tomorrow (Nov 1).

[fivetran-catfritz]

This update has been released on the dbt hub, so closing out this issue.