cli: add: add an --output argument #5284
Conversation
Allow users to add files from outside of the repo. This is a first step toward achieving iterative#4657. References iterative/dvc.org#2101
| if dname and not os.path.isdir(dname): | ||
| os.makedirs(dname) | ||
|
|
||
| shutil.copy2(targets[0], output) |
There was a problem hiding this comment.
This will be really slow with big files/dirs. We could be able to add it without this uneeded copy.
There was a problem hiding this comment.
Also, you are checking that there is only one target in CLI, but not in API (they could be used separately), so targets[0] needs some reassurance.
| assert ret == 1 | ||
|
|
||
| md5, _ = file_md5(fname) | ||
| stage = dvc.add(fname, output="foo")[0] |
There was a problem hiding this comment.
You are actually implementing straight-to-cache functionality #4520 for local files here 🙂 We have a WIP for a general straight-to-remote, where we also use -o in add, they are in no way contradictory though.
| ], | ||
| } | ||
|
|
||
| os.unlink(fname) |
There was a problem hiding this comment.
maybe let's just use tmp_path fixture or smth like that to avoid doing this manually and using file descriptors?
| parser.add_argument( | ||
| "-o", | ||
| "--output", | ||
| type=str, | ||
| metavar="<path>", | ||
| help="Output path for the file, if it lives outside the repo.", | ||
| ) |
There was a problem hiding this comment.
This will clash with --out being introduced in #5198
There was a problem hiding this comment.
It should be --out, but it won't clash otherwise. They are actually the same thing :)
There was a problem hiding this comment.
p.s. looking at this to try to understand iterative/dvc.org/pull/2101
Can someone explain what this does with an example, perhaps?
It's also not obvious to me how this is a step towards #4657 which doesn't seem to involve adding external data (but this does).
Thanks!
There was a problem hiding this comment.
BTW should this maybe be a stand-alone, general option that add can always use (whether the target is external or not)? As disucssed in #5198 (review) 🙂
|
@mrstegeman We've introduced Also, feel free to create a WIP draft for #4657 without creating pre-requisite PRs for now. If we agree on implementation, we could take a step back and just send pre-requisites separately. 🙂 |
Allow users to add files from outside of the repo. This is a first
step toward achieving #4657.
References iterative/dvc.org#2101
❗ I have followed the Contributing to DVC checklist.
📖 If this PR requires documentation updates, I have created a separate PR (or issue, at least) in dvc.org and linked it here.
Thank you for the contribution - we'll try to review it as soon as possible. 🙏