dbterd icon indicating copy to clipboard operation
dbterd copied to clipboard

[FEAT] Use DBT model contracts

Open kevinalh opened this issue 1 year ago • 13 comments

Is your feature request related to a problem? Please describe. DBT introduced model contracts in 1.5.

The constraints field has a way of setting up foreign keys as part of the contract. I don't see a way to use this feature in the dbterd documentation. My models are too big for the relationships tests to run.

Describe the solution you'd like Supporting inferring relationships from DBT model contract native constraints.

If already possible, specifying so in the documentation would be great.

Describe alternatives you've considered I can use the Snowflake dbt_constraints package, but I'd rather use the native DBT constraints feature if that's available. Also dbt_constraints functionality of creating Snowflake constraints is compatible with native model contracts so that's another reason I'd write the native code instead.

I could also use dbt_utils.relationships_where but I'm not sure how (maybe it's possible with the algorithm selection command line syntax?)

I can help writing the code (really like this project!), but want to make sure there's not something I'm missing first. Thanks!

kevinalh avatar Mar 12 '24 23:03 kevinalh

Hi @kevinalh Thanks for the request! Generating ERD based on dbt model contract (or any other metadata) is the missing part here! It will much appreciated if you would create a PR for that 🫶

Could you describe a little bit of your solution (in more user guide detail) here first before development? I am imagining that would a new algo module e.g. dbterd run —algo model_contract, but feel free to suggest it!

I will mark it help-wanted now. Cheers

datnguye avatar Mar 13 '24 01:03 datnguye

👋 @kevinalh Just a check if you are being on this now? Otherwise I can go implement it next week? Thanks

datnguye avatar Mar 16 '24 10:03 datnguye

Hi @datnguye ! Unfortunately got busy with multiple things these days so didn't follow up on this. Feel free to go ahead with the implementation, if you got time I will definitely follow up afterwards as a user since I'm planning on using this feature once it's available :)

kevinalh avatar Mar 16 '24 18:03 kevinalh

Hi @kevinalh Sorry I have to withdraw my words now since I'm very busy in my Paternity Leave period. So I will still mark help_wanted label now until I can secure sometime to deal with this. In the meantime, it's a call for your help on creating PR for this again if possible. Thanks 🙌

datnguye avatar Mar 30 '24 04:03 datnguye

Just giving some suggestions on this enhencement, but feel free to do as you go 🙌

  • It should be the new module as dbterd.adapters.algos.model_contract.py (can be a standalone module)
  • The module must be implemented with at least parse functions (see test_relationship.py module)
    • Func find_related_nodes_by_id: nice to have -- currently used in Python API
    • Func parse_metadata will parse the dbt Cloud metadata - nice to have as well

At the end, it supposed to be able to run below commands:

dbt docs generate
dbterd run --algo model_contract
2024-03-30 17:39:18,550 - dbterd - INFO - Run with dbterd==??? (main.py:54)
2024-03-30 17:39:18,551 - dbterd - INFO - Using dbt artifact dir at: ??? (base.py:74)
2024-03-30 17:39:18,869 - dbterd - INFO - Collected ? table(s) and ? relationship(s) (model_contract.py:???)
2024-03-30 17:39:18,870 - dbterd - INFO - Output saved to C:\Sources\dbterd\target/output.dbml (base.py:198)

datnguye avatar Mar 30 '24 10:03 datnguye

@kevinalh Thanks for making a good issue! I also use dbt model contracts and am very happy to see them supported in dbterd.

@datnguye I am interested in implementing support for dbt model contracts. For my use case it is enough to implement parse for the time being, so I will implement the parse function for dbt model contracts and its tests and send a pull request.

BTW, I just looked at the contents of https://github.com/datnguye/dbterd/pull/103. Is @datnguye already working on the implementation? If not, or if it is too difficult for you to take the time, I will send a pull request. I will be working on it over the weekend, so it could take a little longer...

syou6162 avatar Apr 03 '24 17:04 syou6162

Great @syou6162 please check if @kevinalh started it already and so go ahead! Thanks

I am just starting to look at it and make a refac PR to support it, not implement it actually. I will take the implementation of dbt Cloud if possible only.

datnguye avatar Apr 04 '24 00:04 datnguye

Hi @syou6162 , haven't started working on this either, thanks for offering help with this issue!

kevinalh avatar Apr 04 '24 02:04 kevinalh

@kevinalh @datnguye I tried to get model contracts in this weekend. After much thought, I decided that it is better not to try to support model contracts at this stage. I'll explain why below.

  • The test of relationships uses the ref function of dbt
    • So developers can easily see the relationships between dbt models via depends_on in manifest.json
  • Unfortunately, model contracts' foreign_key does not yet support the ref function
    • ref: https://github.com/dbt-labs/dbt-core/issues/8062
    • Therefore, the foreign_key can only contain the table name directly, and the developer has to find out the relationship between dbt's models the hard way (since depends_on in manifest.json is not available).
  • I have decided that it is better to wait until dbt's model contracts officially support the ref function with foreign_key, although it is possible to work hard to implement that method in dbterd 🙏 .

syou6162 avatar Apr 07 '24 13:04 syou6162

Thanks @syou6162! That’s very useful info 🙌

I think we can still provide some early support to the current model contract’s metadata. The only hard part is to parse the expression, I guess, to know the referenced node. Also I can seem to see one more limitation that the constraints config is just for the model resource type only.

After all, I think that we can still proceed to implement a module to support model contract now with the given known limitations in documentation.

What do you think? @kevinalh @syou6162 Thanks

datnguye avatar Apr 08 '24 04:04 datnguye

This issue has been marked as Stale because it has been open for 180 days with no activity. If you would like the issue to remain open, please comment on the issue or else it will be closed in 7 days.

github-actions[bot] avatar Oct 06 '24 02:10 github-actions[bot]

Sorry for the late reply.

  • https://github.com/dbt-labs/docs.getdbt.com/issues/5983

Since dbt 1.9, it is now possible to use ref in model contracts, so it may be easier to extract column dependencies from artifacts such as manifest.json.

syou6162 avatar Oct 06 '24 23:10 syou6162

I have some time over the New Year holiday, so I think I'll write a pull request to resolve this issue!

syou6162 avatar Jan 01 '25 11:01 syou6162