-
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
get: refactor, clean up and fix dvc get
implementation
#2925
Conversation
Things changed: - both no output and no git file result into PathMissingError now - fail on missing cache or cache download error loudly - removed sqlite bypass, since we use nolock now anyway Refactoring: - streamlined logic and made it similar to Repo.open() one - moved exclusive exception classes to dvc.repo.get
The original agreement was to not unify them for now. "File missing" is accepable for api, but can be improved.
This one should not be supported.
if it failed, then there is no cache so We do distinguish by providing an appropriate cause exception, at least in Repo.open(). |
We already know there is no output with this path. So this is either typo in external out path or erroneous usage of absolute path for a for file
I can't find it, as far as I see both cases are silent. If we raise there then it will be ok. EDIT. Ah, year, there is a warning on missing cache, but then will we get a checkout error?
Both messages are not great:
|
The meta takeout: we have permanent disconnections in what is happening and an error message. I think the reason is error messages texts are disjoint from error handling code. |
So, should we merge it or do we need some clarifications? |
@Suor Merged in a fast-lane mode to unblock import changes. Thank you! 🙂 |
Things changed:
Refactoring:
Things are more complicated here than expected. Still have some issues, things to decide:
FileMissingError
has bad messagedvc get scheme://repo /an/absolute/git/path
when there is no out:P.S. Silent errors on DataCloud.pull() and `RemoteBase.download() are still an issue.