Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Split command #63
Split command #63
Changes from 33 commits
f316613
b377725
0ffcb8e
6ee184e
c7846d0
b2a2ba1
f04f888
cadeae9
34469f4
02d3ef2
1e1f15f
b29c192
131a31d
cd2293b
8d69608
dd614b7
eaef0db
59f2d93
897e869
5eb952a
7089466
e1fb68e
53390a5
e9db0e1
42bc070
6c6e610
ed752ad
dc09510
43efeb4
947889c
7b24191
2628464
57d1aec
9128ad6
de3476b
d907f25
db8c1e3
d7868ad
e8f2791
3db0141
c009159
307cf66
e20f99e
4d5e2df
dda0021
b222ef1
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
What about
exposure
andmetric
/measure
? Are those not listed on purpose?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 logic was to get
nodes
vs the resources that are also other top-level keys in the manifest. Looking with fresh eyes, I would guess there's a dbt-core class that represents this type that we could leverage instead of a list of strings!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.
Since
select
now allows multiple arguments, we cannot have--select
before theproject_name
argument.Instead, we need to order arguments/options specifically
I don't this is a blocker per se. At the very least, documentation should be refined in a follow-up.
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 am of the opinion that
DbtSubprojectCreator
should not allowNone
target_directory
values. Instead, the calling method should be responsible for passing a valid target. This approach will allow us to reduce the complexity of the underlying API/class.This is not something that needs to be tackled here, but rather in a refactor ticket.
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.
that's good feedback, definitely agree -- IIRC the init handles this, but it's definitely not necessary to hide that behavior!