search

datnguye / dbterd

Generate the ERD as a code from dbt artifacts
https://dbterd.datnguyen.de/
MIT License
208 stars 30 forks source link

Support for manifest.json v11 / dbt v1.7 #61

Closed Juniper-vdg closed 10 months ago

Juniper-vdg commented 11 months ago

Describe the bug Similar to #43, dbt manifest.json of version 11 is unable to be parsed by dbterd: Error: Could not open file 'manifest.json': File manifest.json is corrupted, please rebuild

To Reproduce run dbterd run -mv 11

Expected behavior Generation of target ERD script

Desktop (please complete the following information):

Additional context When I upgraded dbt-artifacts-parser in my virtualenv to version 0.5.0 I was able to parse the manifest correctly.

datnguye commented 11 months ago

Thanks @Erik-vdg for raising it! Yes that's the expected behavior where I tried to free the top pin of dbt-artifacts-parser.

I will try to update the documentation for that when available. For now, I will call it discussion and keen to hear your feedback on how we can bypass this automatically.

Thanks

Thanks

Juniper-vdg commented 11 months ago

Thanks @datnguye, I appreciate your comment, and I also am really appreciating the dbterd tool.

From my point of view - I was evaluating the dbterd tool before I would recommend it to my coworkers. I encountered the issue with the dbt-artifacts-parser library on my own and was able to solve it myself. However, some of my colleagues are not very technical and would not be able to solve this on their own.

I would like to propose two options to improve the user experience. Both are not mutually exclusive:

  1. Release a new version of dbterd with the dependency dbt-artifacts-parser pinned to "^0.5".
  2. Enhance the error message when dbterd encounters a manifest version it does not yet know about, suggesting the user to update their version of dbt-artifacts-parser. This is currently emitted when the user runs with the -mv flag, but is not emitted when the user does not provide the -mv flag. I would like the same warning message to be emitted when the -mv flag is not present.

Let me know which option(s) fit in your vision of dbterd. I would be happy to assist by creating a PR for either of these options.

datnguye commented 11 months ago

Cheers @Erik-vdg and yes you are more than welcome to create a PR for it, can you help please? I will try to review and release it as soon as possible.

Regarding to the 2nd point, I am a bit surprised because I implemented this message which is similar to your suggestion. But feel free to make it better and centralized.