-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
import-db: support importing a table #10219
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #10219 +/- ##
==========================================
- Coverage 90.50% 90.47% -0.03%
==========================================
Files 499 499
Lines 37982 38042 +60
Branches 5518 5530 +12
==========================================
+ Hits 34375 34419 +44
- Misses 2965 2980 +15
- Partials 642 643 +1 ☔ View full report in Codecov by Sentry. |
Could be a separate PR, but should we drop the dbt part at this point? |
raise argparse.ArgumentTypeError("Either of --sql or --model is required.") | ||
from dvc.exceptions import InvalidArgumentError | ||
|
||
if self.args.table or self.args.sql: |
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.
Are one of these required?
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.
Yes they are required, and are mutually exclusive.
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.
LGTM but what do you think about dropping the dbt stuff at this point? Seems like it would be useful to document how you could connect your dbt etl to your dvc pipeline, but I don't see much benefit in keeping the dbt-based implementation here.
I was thinking of dropping in the next release, but we could do it now. Will create a separate PR. |
* import-db: support importing a table * unhide the import-db command
I have also made
import-db
command visible in the help message and hidden the dbt-specific options.$ dvc import-db --table customer --conn pgsql
(Note that I haven't added support for downloading a table with dbt's connection profile).