Skip to content
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

get/import/url: handle existing output directory #2610

Merged
merged 2 commits into from
Oct 15, 2019
Merged

Conversation

efiop
Copy link
Contributor

@efiop efiop commented Oct 15, 2019

Currently if you run

dvc get https://github.com/iterative/example-get-started model.pkl --out .

it will nuke your cwd. To fix that we need to properly handle existing
directories, same as usual unix cli utilities do that.

  • Have you followed the guidelines in our
    Contributing document?

  • Does your PR affect documented changes or does it add new functionality
    that should be documented? If yes, have you created a PR for
    dvc.org documenting it or at
    least opened an issue for it? If so, please add a link to it.

iterative/dvc.org#699


Currently if you run

```
dvc get https://github.com/iterative/example-get-started model.pkl --out .
```

it will nuke your cwd. To fix that we need to properly handle existing
directories, same as usual unix cli utilities do that.
@efiop efiop self-assigned this Oct 15, 2019
@efiop efiop added the p0-critical Critical issue. Needs to be fixed ASAP. label Oct 15, 2019
@efiop efiop requested review from Suor, a user and pared October 15, 2019 13:38
@shcheklein
Copy link
Member

@efiop @jorgeorpinel should we create a ticker/clarify the --out behavior in docs?

@efiop
Copy link
Contributor Author

efiop commented Oct 15, 2019

@shcheklein Will do, I didn't click the docs checkbox in the description for that reason :)

@efiop
Copy link
Contributor Author

efiop commented Oct 15, 2019

@shcheklein The docs are written correctly already, it was a bug on our side.

@shcheklein
Copy link
Member

@efiop I'm not sure that it's clear from this ... specify a path (directory and file name) that it one can omit the filename. Also, it's no about files only, it can be a dir name. cc @jorgeorpinel

@efiop
Copy link
Contributor Author

efiop commented Oct 15, 2019

@shcheklein I was more focused on The default value (when this option isn't used) is the current working directory (.) and original file name. But ok, I'll send a PR soon.

@efiop
Copy link
Contributor Author

efiop commented Oct 15, 2019

@efiop efiop merged commit 0bfb3c0 into iterative:master Oct 15, 2019
@efiop efiop deleted the get branch October 15, 2019 19:42
@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented Oct 16, 2019

I was more focused on The default value (when this option isn't used) is the current working directory (.) and original file name....

@efiop is that no longer the case? I don't see that text changed in the PR. Thanks

@efiop
Copy link
Contributor Author

efiop commented Oct 16, 2019

@jorgeorpinel The defaults are the same, so I didn't change that text.

efiop added a commit to efiop/dvc.org that referenced this pull request Oct 16, 2019
jorgeorpinel added a commit to iterative/dvc.org that referenced this pull request Oct 16, 2019
cmd ref: describe updated `--out` behavior in get/import/*-url
Per iterative/dvc#2610
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p0-critical Critical issue. Needs to be fixed ASAP.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants