cmd-reference: document import-db

[skshetry]

No description provided.

[github-actions]

Link Check Report

There were no links to check!

[skshetry]

I have only tested with sqlite, postgres, and snowflake. But it should probably work with all mentioned here (and more).

[dberenbaum]

@skshetry What's the status of this PR?

[dberenbaum]

Can we note that this is experimental and subject to breaking changes?

[dberenbaum]

Left some comments on the doc, but it raised a bigger product question for me that I missed in iterative/dvc#10125. Unlike iterative/dvc#10040, --sql is required here, right? It feels artificial to be limited to a sql query written in the command line. Users likely either want to pull a table, which makes select * seem redundant, or the results of a more complex query.

[skshetry]

Left some comments on the doc, but it raised a bigger product question for me that I missed in iterative/dvc#10125. Unlike iterative/dvc#10040, --sql is required here, right?

We discussed this privately around the time of iterative/dvc#10040 about snapshotting a table or a database, but at that point --sql was not the main focus. It should be easy to add a --table here.

[dberenbaum]

Depends on iterative/dvc#10219

[dberenbaum]

Can we note that this is experimental and subject to breaking changes?

@skshetry Can we add something like this? Otherwise it LGTM.

[skshetry]

@skshetry Can we add something like this? Otherwise it LGTM.

Should we consider it stable now?

[dberenbaum]

I would rather play it safe here, especially considering the discussions in iterative/dvc#10164 and related issues that could impact this. We don't do this so often that it's hard to maintain, so I don't see much downside.

[shcheklein]

https://dvc.org/doc/user-guide/project-structure/dvc-files - do we need to update some of these files?

also this https://dvc.org/doc/command-reference/update

[dberenbaum]

https://dvc.org/doc/user-guide/project-structure/dvc-files - do we need to update some of these files?

also this https://dvc.org/doc/command-reference/update

Yes, good catch

[skshetry]

dvc.org/doc/user-guide/project-structure/dvc-files - do we need to update some of these files?

also this dvc.org/doc/command-reference/update

Since we consider this as experimental, I'd avoid updating other files for now.

[dberenbaum]

Since we consider this as experimental, I'd avoid updating other files for now.

That's fine with me as long as we have an issue to track it so we don't forget.

[dberenbaum]

@skshetry Could you resolve the rest of the comments so we can merge?

[shcheklein]

Since we consider this as experimental, I'd avoid updating other files for now.

could you folks share some light on this idea? what is the expectation to make a proper feature?

also, can we just put a note / admon on the top of the cmd ref pages? it would be more explicit I guess

(not a blocker - just the first time I see that we are being conservative with updates to docs and just curios about the procedure you have in mind)

[skshetry]

could you folks share some light on this idea? what is the expectation to make a proper feature?

See #5033 (comment).

We want to see if we can fit this into datasets or datasets-like interface, since users are likely to read from their database in their scripts rather than snapshotting into a csv file.
But I don't think it is either-or here. (But snapshotting users may be smaller than streaming).

[skshetry]

Ideally, this should be:

Suggested change

	[--sql SQL | --table TABLE] [--conn CONN]
	(--sql SQL | --table TABLE) --conn CONN

But auto-linking seems to be broken with this.

[shcheklein]

I think we should change it in the DVC itself also. We don't use this style. I mean all uppercase CONN or TABLE. Pleas check other commands.

If this (--sql SQL | --table TABLE) is correct (why?) - we should also fix first on the DVC side - and adjust auto-linker here, I hope it's not that complicated

but let's check if we do this in some other DVC commands

[skshetry]

They are a mutually-exclusive argument, and one of them is required. I can change the casings of TABLE/SQL with metavar (uppercased name is just default, and we do those in many places when we are lazy).

[skshetry]

I have changed this in the docs. Will handle this in DVC separately.

[shcheklein]

and we do those in many places when we are lazy

are there examples? I checked a few commands and did't find it.

anyways, I think we need to update it on the DVC side for all the commands

They are a mutually-exclusive argument, and one of them is required.

good point, then let's change on the DVC side first and we can fix the website later ... DVC is more important I think

[skshetry]

usage: dvc experiments run [-h] [-q | -v] [-f] [-i] [-s] [-p] [-P] [-R] [--downstream] [--force-downstream] [--pull]
                           [--allow-missing] [--dry] [-k] [--ignore-errors] [-n <name>] [-S [<filename>:]<param_name>=<param_value>]
                           [--queue] [--run-all] [-j <number>] [--temp] [-C COPY_PATHS] [-m MESSAGE]
                           [targets ...]

See [-m MESSAGE] and [-c COPY_PATHS]. It's just the default from argparse, so it's easy to miss here. There are other commands like du/artifacts get/exp save/fetch/ls-url/ls/get/import, etc that does this. :)

[skshetry]

Fixed this in dvc by iterative/dvc#10226.

[skshetry]

That's fine with me as long as we have an issue to track it so we don't forget.

Created #5087.

[dberenbaum]

Thanks @skshetry! LGTM.