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

[FEAT] Use DBT model contracts #95

Open
kevinalh opened this issue Mar 12, 2024 · 10 comments
Open

[FEAT] Use DBT model contracts #95

kevinalh opened this issue Mar 12, 2024 · 10 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@kevinalh
Copy link

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!

@datnguye
Copy link
Owner

datnguye commented Mar 13, 2024

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 datnguye added enhancement New feature or request help wanted Extra attention is needed and removed triage labels Mar 13, 2024
@datnguye
Copy link
Owner

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

@kevinalh
Copy link
Author

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 :)

@datnguye
Copy link
Owner

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
Copy link
Owner

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)

@syou6162
Copy link
Contributor

syou6162 commented Apr 3, 2024

@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 #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...

@datnguye
Copy link
Owner

datnguye commented Apr 4, 2024

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.

@kevinalh
Copy link
Author

kevinalh commented Apr 4, 2024

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

@syou6162
Copy link
Contributor

syou6162 commented Apr 7, 2024

@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
  • 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 🙏 .

@datnguye
Copy link
Owner

datnguye commented Apr 8, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants