Remove UrlNotDvcRepoError from exc for dvc.api#1763
Conversation
| ## Exceptions | ||
|
|
||
| - `dvc.api.UrlNotDvcRepoError` - `repo` is not a DVC project. | ||
|
|
There was a problem hiding this comment.
It returns a whole kinds of exceptions:
PathMissingErrorOutputNotFoundError(local) /NoOutputInExternalRepoError(remote repo)NoRemoteError(local) /NoRemoteInExternalRepoError(remote repo)- Possibly
OutputNotFoundErroragain.
There was a problem hiding this comment.
But, something to fix on the core repo.
There was a problem hiding this comment.
So is UrlNotDvcRepoError not returned then? What do you recommend the docs say here for now? Whatever best represents the current behavior.
There was a problem hiding this comment.
Let's leave it as-is for now, we should better fix this in the core by throwing only certain sets of exceptions, only ApiError perhaps?
There was a problem hiding this comment.
Well, we already merged this PR where you removed that line. Should we put it back then? Or by "as-is" do you mean having nothing here is OK?
we should better fix this in the core by throwing only certain sets of exceptions, only ApiError perhaps?
Is there a core issue for that?
Thanks
jorgeorpinel
left a comment
jorgeorpinel
left a comment
There was a problem hiding this comment.
Sorry for the delay and thanks for this @skshetry
Trusting you on this one, feel free to merge.
| ## Exceptions | ||
|
|
||
| - `dvc.api.UrlNotDvcRepoError` - `repo` is not a DVC project. | ||
|
|
There was a problem hiding this comment.
So is UrlNotDvcRepoError not returned then? What do you recommend the docs say here for now? Whatever best represents the current behavior.
4 participants
We added support for subrepos in treeverse/dvc#4465, due to which it made
UrlNotDvcRepoErrorinvalid and imprecise. So, we made a breaking change for the exceptions raised.There's a lot of things that are wrong with exceptions for
dvc.api, which I don't want to fix them here but on the core side (raiseApiErroreverywhere, perhaps?).❗ Please read the guidelines in the Contributing to the Documentation list if you make any substantial changes to the documentation or JS engine.
🐛 Please make sure to mention
Fix #issue(if applicable) in the description of the PR. This causes GitHub to close it automatically when the PR is merged.Please choose to allow us to edit your branch when creating the PR.
Thank you for the contribution - we'll try to review it as soon as possible. 🙏