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

[CT-894] [Feature] Migrate array cross-db macros from dbt-utils #5520

Closed
1 task done
joellabes opened this issue Jul 25, 2022 · 4 comments
Closed
1 task done

[CT-894] [Feature] Migrate array cross-db macros from dbt-utils #5520

joellabes opened this issue Jul 25, 2022 · 4 comments
Assignees
Labels
enhancement New feature or request Team:Adapters Issues designated for the adapter area of the code utils Cross-database building blocks
Milestone

Comments

@joellabes
Copy link
Contributor

Is this your first time opening an issue?

Describe the Feature

A follow-on to #4813 and its follow-ons - dbt-labs/dbt-utils#595 added some new array-y macros:

  • array_append
  • array_concat
  • array_construct
  • cast_array_to_string

These have missed the bus for inclusion in Core v1.2, but should be added for 1.3.

FYI: My ongoing plan for any future macros is detailed here - in short, if someone need a new cross-db macro, they add it to their own package until the next minor version of dbt Core, at which point they can swap to the official one. If you have any pushback on that strategy, please let me know!

Describe alternatives you've considered

  • Instead of being added to dbt Core, they migrate back to the https://github.com/dbt-labs/dbt-project-evaluator project. They seem useful and applicable in multiple contexts so I don't think this is the right move.
  • They could stay in utils forever, but it sorta defeats the purpose of moving all of their siblings out.

Who will this benefit?

No response

Are you interested in contributing this feature?

I'm sorta hoping Doug might 😅

Anything else?

No response

@joellabes joellabes added enhancement New feature or request triage labels Jul 25, 2022
@github-actions github-actions bot changed the title [Feature] Add some straggler cross-db macros [CT-894] [Feature] Add some straggler cross-db macros Jul 25, 2022
@jtcohen6
Copy link
Contributor

FYI: My ongoing plan for any future macros is dbt-labs/dbt-utils#487 (reply in thread) - in short, if someone need a new cross-db macro, they add it to their own package until the next minor version of dbt Core, at which point they can swap to the official one. If you have any pushback on that strategy, please let me know!

This strategy makes sense to me. I think low-level "cross-db" macros should skip over dbt-utils entirely. We may decide that a macro is operating at a higher level of complexity / utility, and that dbt-core isn't the right place for it, at which point we can try to find another long-term home.

I think our requirements for a cross-db macro to make its way into dbt-core should be:

  • It must be implemented on the five adapter plugins that dbt Labs maintains
  • It must have functional testing

Given that adding one macro requires opening PRs across several adapter plugin repos, it is nicer to batch up many at once. In theory, though, these could be good opportunities for external contribution.

I've just created a new label, utils, to keep track of this and other follow-on work from the cross-db macro migration.

@jtcohen6 jtcohen6 added utils Cross-database building blocks and removed triage labels Jul 25, 2022
@jtcohen6 jtcohen6 added this to the v1.3 milestone Jul 25, 2022
@leahwicz leahwicz added the Team:Adapters Issues designated for the adapter area of the code label Jul 26, 2022
@leahwicz
Copy link
Contributor

@dbeatty10 let's sync if this is something you have bandwidth for and would like to do. We could even get you paired with an Adapters person to do it together if that makes sense.

@dbeatty10
Copy link
Contributor

Count me in @leahwicz

I can get the PRs opened and the macros moved over. The thing that I would welcome the most help with:

  • robust testing

@leahwicz
Copy link
Contributor

Thanks @dbeatty10! I'm going to assign this to you then and then once you start working on this and looking for testing help, let me know and we can coordinate

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Team:Adapters Issues designated for the adapter area of the code utils Cross-database building blocks
Projects
None yet
Development

No branches or pull requests

4 participants