-
Notifications
You must be signed in to change notification settings - Fork 79
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
Fix issue where sources nodes with tags were excluded from graph viz #139
Conversation
Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: John Lynch.
|
2 similar comments
Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: John Lynch.
|
Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: John Lynch.
|
Really nice catch @jplynch77!! I think this logic is pretty funky. My bet is that this logic came from the fact that
but I don't feel super strongly about that. I wonder if tagged snapshots or seeds would show up with this current implementation. Maybe we can test that out and decide what the right implementation is? Either way, I think you found exactly the right line of code to modify! Let me know if you need a hand with the CLA-bot thing. You might just need to rebase your earlier commits to make sure that every commit is signed as your user. |
b293cf7
to
96860c0
Compare
Thanks for the review @drewbanin ! That's a good point about capturing tags from other And thanks for the tip on rebasing - that seemed to do the trick. |
@drewbanin did some testing and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM! Thanks for the PR on this one @jplynch77 🎉
resolves #93
Description
This resolves an issue where source nodes with tags were not showing up in the graph visualization. The root problem seems to be that tags from sources are not added to
all_tags
inmain/index.js
. This causesselectNodes()
inselector_methods.js
to exclude sources with tags. Specifically, a source node with a tag ends up hitting the(!matched_tags && !matched_untagged)
condition inThis change also makes tags defined only on sources available in tags drop-down in the graph viz.
Checklist
CHANGELOG.md
and added information about my change to the "dbt next" section.