Repository: iterative/dvc
Pull request: cli: remove: fix error message for the case when file is not under control (#4497)
Partially fix #4497 

* [x] ❗ I have followed the [Contributing to DVC](https://dvc.org/doc/user-guide/contributing/core) checklist.

* [x] 📖 If this PR requires [documentation](https://dvc.org/doc) updates, I have created a separate PR (or issue, at least) in [dvc.org](https://github.com/iterative/dvc.org) and linked it here.

Thank you for the contribution - we'll try to review it as soon as possible. 🙏


Problem statement:
remove one of many stage outputs: misleading error message
Users cannot remove a data file if it is only one output of many for a stage. It is a valid behavior that needs a better error message:

```
$ dvc remove model.h5
ERROR: failed to remove 'model.h5' - "Stage 'model.h5' not found inside 'dvc.yaml' file"
```

The pipeline:
```
$ cat dvc.yaml
stages:
  train:
    cmd: python train.py
    deps:
    - users.csv
    - train.py
    - mymodel.py
    params:
    - epochs
    - opt
    - log_file
    - dropout
    outs:
    - logs
    - model.h5       # <-- trying to remove this one
    metrics:
    - summary.json:
        cache: false
    plots:
    - logs.csv:
        cache: false
```

Hints from issue discussion:
Also, formatting issues:
1. The prefix ` failed to remove 'model.h5' -` is not needed. Too obvious.
2. Quoted error message `"Stage 'model.h5' not ..."`
@dmpetrov, we don't support removing a `.dvc` file by its output name yet. The target should be either a `.dvc` file or a name of the stage.
Another user ran into it https://discord.com/channels/485586884165107732/485596304961962003/798834932217937970

Commit message: cli: remove: fix error message for the case when file is not under control (#4497)

2nd round of fixes after code review

Patch to review:
diff --git a/dvc/command/remove.py b/dvc/command/remove.py
index c2dab5e7a1..8b2ef4d8d7 100644
--- a/dvc/command/remove.py
+++ b/dvc/command/remove.py
@@ -13,8 +13,8 @@ def run(self):
         for target in self.args.targets:
             try:
                 self.repo.remove(target, outs=self.args.outs)
-            except DvcException:
-                logger.exception(f"failed to remove '{target}'")
+            except DvcException as exc:
+                logger.error(exc)
                 return 1
         return 0
 
diff --git a/dvc/stage/exceptions.py b/dvc/stage/exceptions.py
index 84d5a86564..a98bfcd56a 100644
--- a/dvc/stage/exceptions.py
+++ b/dvc/stage/exceptions.py
@@ -80,7 +80,7 @@ def __init__(self, missing_files):
         super().__init__(msg)
 
 
-class StageNotFound(KeyError, DvcException):
+class StageNotFound(DvcException):
     def __init__(self, file, name):
         self.file = file.relpath
         self.name = name


Code review comments:
diff_hunk: @@ -80,7 +80,7 @@ def __init__(self, missing_files):
         super().__init__(msg)
 
 
-class StageNotFound(KeyError, DvcException):
+class StageNotFound(DvcException):
path: dvc/stage/exceptions.py
text: @user1:
Why do we need to remove `KeyError`?

@author:
```KeyError``` was the reason why we have extra quotes in the initial messages. To fix that, I've removed it. But before I double-checked the usage of ```StageNotFound```, and didn't find a reason for inheritance from ```KeyError```. So it looked safe for me to remove that inheritance.

@user1:
Internally, StageLoader still works like a dictionary. So, I'd still prefer that we keep the semantics of `StageNotFound` being a KeyError as well.

Here's one example:
https://github.com/iterative/dvc/blob/273518afab02a7b229fc0ef0eba43a556fb80a44/dvc/repo/stage.py#L184

We can get around the quoting behaviour by using a custom `__str__` implementation.
```suggestion
    __str__ = DvcException.__str__
```



@author:
@user1 Done