Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hard code dispatch namespaces for fivetran_utils? #3403

Merged
merged 2 commits into from
Jun 1, 2021

Conversation

jtcohen6
Copy link
Contributor

Picks up from #3363

The _get_utils_namespaces() macro in fivetran_utils (source) works just slightly differently from the other get-namespace macros: It includes dbt_utils, alongside itself, in the list of default packages to search in. So folks with Fivetran packages installed might see this error upon upgrading to 0.19.2 / 0.20.0:

Encountered an error:
Compilation Error
  In dispatch: No macro named 'datediff' found
      Searched for: 'fivetran_utils.snowflake__datediff', 'fivetran_utils.default__datediff'

The most straightforward fix for this is just to set this variable in their project, and everything will work as anticipated:

vars:
  fivetran_utils_dispatch_list: ['dbt_utils']

This is closer to the syntax that users will need to have going forward:

dispatch:
  - macro_namespace: fivetran_utils
    search_order: ['dbt_utils', 'fivetran_utils']

At the same time, I want to take backwards compatibility really seriously, since this is otherwise a breaking change in a patch release. So this PR would shamelessly hard code backwards compatibility for that specific behavior.

I don't much like this:dbt_utils isn't actually a dependency of fivetran_utils, so it's conceivable that this might raise an error if dbt_utils isn't installed alongside it. That's true parity with current behavior, though.

I could really go either way on this one!

@cla-bot cla-bot bot added the cla:yes label May 29, 2021
@jtcohen6 jtcohen6 requested a review from gshank June 1, 2021 12:36
@jtcohen6 jtcohen6 force-pushed the fix/backward-compat-fivetran-utils branch from be89d86 to 688436d Compare June 1, 2021 13:40
@jtcohen6
Copy link
Contributor Author

jtcohen6 commented Jun 1, 2021

[skip cii]

🤦 Well, it was worth a shot. Bound to happen anyway given the rebase.

@jtcohen6 jtcohen6 merged commit 1e5a787 into develop Jun 1, 2021
@jtcohen6 jtcohen6 deleted the fix/backward-compat-fivetran-utils branch June 1, 2021 14:58
@jtcohen6 jtcohen6 mentioned this pull request Jun 1, 2021
2 tasks
gshank pushed a commit that referenced this pull request Jun 1, 2021
* Hard code for fivetran_utils

* Add changelog entry [skip cii]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants