Conversation
|
👋 @IDoneShaveIt |
| return [] | ||
| # When nodes are matched, ls command returns strings of the node names. | ||
| else: | ||
| return command_outputs |
There was a problem hiding this comment.
Isn't it better to also return the JSON-loaded outputs here?
There was a problem hiding this comment.
If nodes are matched, the output of dbt will be:
"node1\nnode2\n"
So I don't see why we should return it like that as well.
When there are no nodes the output is always the same and our warning explains the same thing so it is redundant and will never be used IMO.
WDYT?
There was a problem hiding this comment.
I mean something else :)
It's good that you split the rows, but I don't get why you call try_load_json and then return the non-json loaded values - why not return the output of try_load_json?
There was a problem hiding this comment.
Because I only use try_load_json to understand that I have no nodes matches.
Those json has no additional information, and has no use.
I prefer keeping it all simple and the same typing wise:
If there are matches, return a list of the matches.
If there are no matches, log an error and return an empty list.
There is no value by returning 2 dicts that has message that said that there are no node matches and then parse it outside the function.
There was a problem hiding this comment.
Ah OK, I see what you mean
I misunderstood this. Sababa.
| "If specified, the target will be used for your dbt project." | ||
| "Else, the --profile-target will be used.", | ||
| )(func) | ||
| func = click.option( |
There was a problem hiding this comment.
I don't think it's critical for now but I wonder if people will expect multiple selectors to work as well.
Can be added to a future version if needed though.
There was a problem hiding this comment.
I think that dbt supports only a single selector (when passing more then one it uses the latest one), so I think that we should see the usage of it and wait for users' requests 🙂
There was a problem hiding this comment.
They support multiple:
But again - I don't see it as critical for now.
There was a problem hiding this comment.
Oh I was thinking about providing multiple tags / owners.
I think it could be step 2
Co-authored-by: Itamar Hartstein <haritamar@gmail.com>
Co-authored-by: Itamar Hartstein <haritamar@gmail.com>
2 participants
Support filtering alerts using --select argument.
In case project dir is provided, we use dbt ls command to filter the nodes.
Otherwise we support manual filtering over tag / owner / model