-
Notifications
You must be signed in to change notification settings - Fork 170
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
Add dbt clone operator #1326
base: main
Are you sure you want to change the base?
Add dbt clone operator #1326
Conversation
Deploying astronomer-cosmos with Cloudflare Pages
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1326 +/- ##
==========================================
+ Coverage 95.85% 95.88% +0.02%
==========================================
Files 67 67
Lines 3983 4009 +26
==========================================
+ Hits 3818 3844 +26
Misses 165 165 ☔ View full report in Codecov by Sentry. |
e833883
to
8af04da
Compare
1187827
to
2ecf1e1
Compare
class DbtCloneAwsEksOperator(DbtAwsEksBaseOperator, DbtCloneKubernetesOperator): | ||
""" | ||
Executes a dbt core clone command. | ||
""" | ||
|
||
def __init__(self, *args: Any, **kwargs: Any) -> None: | ||
super().__init__(*args, **kwargs) |
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 value is the constructor giving us? Would it make sense to do something like this:
class DbtCloneAwsEksOperator(DbtAwsEksBaseOperator, DbtCloneKubernetesOperator): | |
""" | |
Executes a dbt core clone command. | |
""" | |
def __init__(self, *args: Any, **kwargs: Any) -> None: | |
super().__init__(*args, **kwargs) | |
class DbtCloneAwsEksOperator(DbtAwsEksBaseOperator, DbtCloneKubernetesOperator): | |
""" | |
Executes a dbt core clone command. | |
""" | |
pass |
Executes a dbt core clone command. | ||
""" | ||
|
||
def __init__(self, *args: Any, **kwargs: Any): |
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.
The same feedback given on https://github.com/astronomer/astronomer-cosmos/pull/1326/files#r1843904192 applies here and in the other DbtClone
operators
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.
@pankajastro it's great to have you back!
Would it make sense to add the example DAG you used to test as one of Cosmos' integration DAGs?
Please, could you add documentation about this feature? I believe we may be lacking docs on how people can use DbtBuild
operators - we could cover both in the same place.
Last but not least, it may be worth to have a test related to full refresh. Some Cosmos operators support full refresh, others don't. It seems clone supports this, so it would be great to make sure the interfaces are consistent with running models and seeds.
1b4bde3
to
629fa82
Compare
629fa82
to
8dcf654
Compare
Description
This PR introduces the DbtCloneOperator. For more details, refer to the dbt documentation: https://docs.getdbt.com/reference/commands/clone.
Testing
Airflow DAG
DBT Profile
Airflow DAG Run
BQ data WH
Related Issue(s)
closes: #1268
closes: #878
Breaking Change?
No
Limitation
dbt clone
command was introduced in dbt-core 1.6.0, so this feature is only available to users with dbt-core version 1.6 or higher https://github.com/dbt-labs/dbt-core/blob/1.6.latest/CHANGELOG.mdChecklist