-
Notifications
You must be signed in to change notification settings - Fork 121
fix: filter dbt models to upload only current project #793
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
base: master
Are you sure you want to change the base?
Conversation
|
Thanks for your contribution! |
|
I haven't seen in contributing guide a section about flag. Could you explain to me what is required ? |
|
Since it changes the behavior, we want it to be an option that the user actively chooses. Either by configuration, or by an argument that the program receives. With the default being set to the original behavior. |
|
This does not change current behavior. |
|
This pull request is stale because it has been open for too long with no activity. |
elazarlachkar
left a comment
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.
Hi @cedric-orange!
I'm sorry for the delayed response.
Your PR looks good to me and I would like to merge it, I just commented with one question.
Could you look at it, and also update your branch to the latest elementary-data:master for the CI to pass?
Also, is that relevant also to groups and metrics artifacts?
| {% set relation = elementary.get_elementary_relation('dbt_columns') %} | ||
| {% if execute and relation %} | ||
| {% set tables = graph.nodes.values() | list + graph.sources.values() | list %} | ||
| {% set tables = graph.nodes.values() | selectattr("package_name", "equalto", project_name) | list + graph.sources.values() | list %} |
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.
I'm not familiar with project_name, but noticed in the dbt docs that it is accessed using the context (context[project_name]). Does this work without it?
When you are using some tools like dbt-loom,
graph.nodes.values()returns nodes from multiples project.dbt-loom works by fetching public model definitions from your dbt artifacts, and injecting those models into your dbt project.
The goal of this PR is to only retrieve nodes from the current project using project_name selection to avoid uploading of unnecessary information.