Add support for ephemeral models#90
Conversation
owlas
left a comment
owlas
left a comment
There was a problem hiding this comment.
looking good! just a request to move the tag feature to a separate PR
| if node.resource_type == 'model' and node.config['materialized'] != 'ephemeral' | ||
| ] | ||
|
|
||
| if tag is None: |
There was a problem hiding this comment.
could you remove the tag feature from this PR and we can address it separately?
There was a problem hiding this comment.
How do you mean, sorry?
This line isn't doing anything new, it's just some light refactoring for clarity (lines 46-48 previously did this filtering).
I guess I have changed the order in which things happen - previously, an error would have been raised for any model that didn't have a name attribute, whereas now (because I've put the filtering before the for-loop) only selected models (ones where the tag matches) that don't have a name attribute will throw the error, the rest will be ignored.
Can you confirm that this behavioural change is what you want me to revert?
There was a problem hiding this comment.
@owlas I assumed that this is what you wanted, I've fiddled with it to remove the behavioural change, which I will include in a new PR shortly.
There was a problem hiding this comment.
All done - there are now three PRs in a stack:
- Add support for ephemeral models #90 (this one)
- then Filter models on tags before checking empties #92 (contains the separated tag filter change)
- then Warn about catalog manifest discrepancy #91 (separate work, but stacked on top because I couldn't develop locally without first solving these issues with ephemeral models and tag-based filtering)
PaddyAlton
force-pushed
the
57-issue-parsing-ephemeral-models
branch
from
0dd93fb to
5552ab2
Compare
|
Hi @owlas - just checking whether my revisions following your review got lost in the churn? There are three small PRs in a stack:
No particular rush, but thought I'd check this was on your radar. |
|
@owlas - just a gentle nudge, any chance of merging these contributions? |
2 participants
This PR fixes the
dbt2lookertool such that ephemeral models do not break it (see issue #57).It is inspired by earlier suggestions by @boludo00 and @mariana-s-fernandes , with minor modifications.