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

[WIP] dvc: wrap checkout error as download error #2608

Closed
wants to merge 2 commits into from

Conversation

Suor
Copy link
Contributor

@Suor Suor commented Oct 14, 2019

This will happen on missing caches and similar to download error in its
effect.

We might want to think of making missing caches an error. That is, however, not an easy thing to do since we will need to postpone/collect those errors in fetch and turn into a warning in status.

dvc/repo/fetch.py Outdated Show resolved Hide resolved
This will happen on missing caches and similar to download error in its
effect.
@Suor Suor changed the title dvc: wrap checkout error as download error in Repo.fetch() [WIP] dvc: wrap checkout error as download error in Repo.fetch() Oct 22, 2019
@Suor Suor changed the title [WIP] dvc: wrap checkout error as download error in Repo.fetch() [WIP] dvc: wrap checkout error as download error Oct 22, 2019
@Suor
Copy link
Contributor Author

Suor commented Oct 22, 2019

So the issue is when we pull an imported thing and that doesn't exist (repo or its cache was deleted) then we get CheckoutError instead of DownloadError.

We can wrap to fix it, however, the core issue is that DataCloud.pull() is silently ok with missing files. So we might want to change that instead.

@efiop
Copy link
Contributor

efiop commented Nov 15, 2019

@Suor So do we need this PR?

@Suor
Copy link
Contributor Author

Suor commented Nov 15, 2019

Do we worry about a wrong error message? If we do then we should. The message now is:

Checkout failed for following targets:
     <some file name>
Did you forget to fetch?

While that might be caused by an output missing in a cache and silently skipped during fetch or that someone removed/moved source repo cache folder.

This PR only shows the problem via failing test, but doesn't fix it. We might add a solution on top.

@efiop
Copy link
Contributor

efiop commented Nov 18, 2019

@Suor Didn't mean to push, just was checking what is the status here. Ok, let's include it into the next sprint then to fix it.

@Suor
Copy link
Contributor Author

Suor commented Dec 1, 2019

Looks like this is low priority for everyone including myself. Closing this for now, will handle when we run into it or with some related refactoring later.

@Suor Suor closed this Dec 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants