Repository: iterative/dvc
Pull request: exp push: improve ui
Closes https://github.com/iterative/dvc/issues/9280.

Problem statement:
exp push: clarify in UI what has been pushed to DVC and Git remotes
`dvc exp push` is doing a lot, potentially pushing multiple experiments to Git, a DVC remote, and Studio. The output doesn't always make clear what happened, what was successful, and what failed.

**Example 1**: When the dvc push fails, the command errors out with no additional info even though the git push succeeded:

```bash
$ dvc exp push origin crank-prat
ERROR: unexpected error - [Errno 22] Bad Request: An error occurred (400) when calling the HeadObject operation: Bad Request
```

We should still show what was pushed to git even if we throw this error.

**Example 2**: When some experiments have already been pushed and some haven't, the message could better clarify that some of the experiments were already pushed:

```bash
$ dvc exp push --no-cache origin handy-puns lardy-vara
Pushed experiment 'lardy-vara' to Git remote 'origin'.
To push cached outputs for this experiment to DVC remote storage,re-run this command without '--no-cache'.
```

We should include that experiment 'handy-puns' was already pushed to that git remote.

**Example 3**: When the experiment was previously pushed with `--no-cache`, pushing again with the cache will take time to upload to the dvc remote but then report nothing was pushed:

```bash
$ dvc exp push origin lardy-vara
No experiments to push.
```

We should include the push output, like "9 files pushed to DVC remote 'storage'."

**Example 4**: Even when the experiment is successfully pushed, it would help to clarify that it was pushed not only to the Git remote but also to the DVC remote:

```bash
$ dvc exp push origin lushy-wool
Pushed experiment 'lushy-wool' to Git remote 'origin'.
```

This could use the same kind of output suggested for example 3.

Hints from issue discussion:

Commit message: exp push: improve ui

Patch to review:
diff --git a/dvc/commands/experiments/push.py b/dvc/commands/experiments/push.py
index 239971a371..c5bad388ca 100644
--- a/dvc/commands/experiments/push.py
+++ b/dvc/commands/experiments/push.py
@@ -1,5 +1,6 @@
 import argparse
 import logging
+from typing import Any, Dict
 
 from dvc.cli import completion
 from dvc.cli.command import CmdBase
@@ -18,34 +19,60 @@ def raise_error_if_all_disabled(self):
                 "`--rev` or `--all-commits` flag."
             )
 
-    def run(self):
+    @staticmethod
+    def log_result(result: Dict[str, Any], remote: str):
         from dvc.utils import humanize
 
-        self.raise_error_if_all_disabled()
+        def join_exps(exps):
+            return humanize.join([f"[bold]{e}[/]" for e in exps])
 
-        result = self.repo.experiments.push(
-            self.args.git_remote,
-            self.args.experiment,
-            all_commits=self.args.all_commits,
-            rev=self.args.rev,
-            num=self.args.num,
-            force=self.args.force,
-            push_cache=self.args.push_cache,
-            dvc_remote=self.args.dvc_remote,
-            jobs=self.args.jobs,
-            run_cache=self.args.run_cache,
-        )
-        if pushed_exps := result.get("pushed"):
-            ui.write(
-                f"Pushed experiment {humanize.join(map(repr, pushed_exps))} "
-                f"to Git remote {self.args.git_remote!r}."
+        if diverged_exps := result.get("diverged"):
+            exps = join_exps(diverged_exps)
+            ui.warn(
+                f"Local experiment {exps} has diverged "
+                "from remote experiment with the same name. "
+                "To override the remote experiment re-run with '--force'."
             )
-        else:
+        if uptodate_exps := result.get("up_to_date"):
+            verb = "are" if len(uptodate_exps) > 1 else "is"
+            exps = join_exps(uptodate_exps)
+            ui.write(f"Experiment {exps} {verb} up to date.", styled=True)
+        if pushed_exps := result.get("success"):
+            exps = join_exps(pushed_exps)
+            ui.write(f"Pushed experiment {exps} to Git remote {remote!r}.", styled=True)
+        if not uptodate_exps and not pushed_exps:
             ui.write("No experiments to push.")
 
         if project_url := result.get("url"):
             ui.write("[yellow]View your experiments at", project_url, styled=True)
 
+        if uploaded := result.get("uploaded"):
+            stats = {"uploaded": uploaded}
+            ui.write(humanize.get_summary(stats.items()))
+
+    def run(self):
+        from dvc.repo.experiments.push import UploadError
+
+        self.raise_error_if_all_disabled()
+
+        try:
+            result = self.repo.experiments.push(
+                self.args.git_remote,
+                self.args.experiment,
+                all_commits=self.args.all_commits,
+                rev=self.args.rev,
+                num=self.args.num,
+                force=self.args.force,
+                push_cache=self.args.push_cache,
+                dvc_remote=self.args.dvc_remote,
+                jobs=self.args.jobs,
+                run_cache=self.args.run_cache,
+            )
+        except UploadError as e:
+            self.log_result(e.result, self.args.git_remote)
+            raise
+
+        self.log_result(result, self.args.git_remote)
         if not self.args.push_cache:
             ui.write(
                 "To push cached outputs",
diff --git a/dvc/repo/experiments/push.py b/dvc/repo/experiments/push.py
index 160970c472..77c7fd9b0d 100644
--- a/dvc/repo/experiments/push.py
+++ b/dvc/repo/experiments/push.py
@@ -14,10 +14,10 @@
 from funcy import compact, group_by
 from scmrepo.git.backend.base import SyncStatus
 
+from dvc.exceptions import DvcException
 from dvc.repo import locked
 from dvc.repo.scm_context import scm_context
 from dvc.scm import Git, TqdmGit, iter_revs
-from dvc.ui import ui
 from dvc.utils import env2bool
 
 from .exceptions import UnresolvedExpNamesError
@@ -30,6 +30,12 @@
 logger = logging.getLogger(__name__)
 
 
+class UploadError(DvcException):
+    def __init__(self, msg, result):
+        self.result = result
+        super().__init__(msg)
+
+
 def notify_refs_to_studio(
     repo: "Repo", git_remote: str, **refs: List[str]
 ) -> Optional[str]:
@@ -104,23 +110,26 @@ def push(  # noqa: C901
                 exp_ref_set.update(ref_info_list)
 
     push_result = _push(repo, git_remote, exp_ref_set, force)
-    if push_result[SyncStatus.DIVERGED]:
-        diverged_refs = [ref.name for ref in push_result[SyncStatus.DIVERGED]]
-        ui.warn(
-            f"Local experiment '{diverged_refs}' has diverged from remote "
-            "experiment with the same name. To override the remote experiment "
-            "re-run with '--force'."
-        )
+
+    refs = {
+        status.name.lower(): [ref.name for ref in ref_list]
+        for status, ref_list in push_result.items()
+    }
+    result: Dict[str, Any] = {**refs, "uploaded": 0}
+
     if push_cache:
         push_cache_ref = (
             push_result[SyncStatus.UP_TO_DATE] + push_result[SyncStatus.SUCCESS]
         )
-        _push_cache(repo, push_cache_ref, **kwargs)
 
-    refs = push_result[SyncStatus.SUCCESS]
-    pushed_refs = [str(r) for r in refs]
+        try:
+            result["uploaded"] = _push_cache(repo, push_cache_ref, **kwargs)
+        except Exception as exc:  # noqa: BLE001, pylint: disable=broad-except
+            raise UploadError("failed to push cache", result) from exc
+
+    pushed_refs = [str(r) for r in push_result[SyncStatus.SUCCESS]]
     url = notify_refs_to_studio(repo, git_remote, pushed=pushed_refs)
-    return {"pushed": [ref.name for ref in refs], "url": url}
+    return {**result, "url": url}
 
 
 def _push(
@@ -161,10 +170,10 @@ def _push_cache(
     dvc_remote: Optional[str] = None,
     jobs: Optional[int] = None,
     run_cache: bool = False,
-):
+) -> int:
     if isinstance(refs, ExpRefInfo):
         refs = [refs]
     assert isinstance(repo.scm, Git)
     revs = list(exp_commits(repo.scm, refs))
     logger.debug("dvc push experiment '%s'", refs)
-    repo.push(jobs=jobs, remote=dvc_remote, run_cache=run_cache, revs=revs)
+    return repo.push(jobs=jobs, remote=dvc_remote, run_cache=run_cache, revs=revs)
diff --git a/tests/func/experiments/test_remote.py b/tests/func/experiments/test_remote.py
index c9df12ca04..2281db83f5 100644
--- a/tests/func/experiments/test_remote.py
+++ b/tests/func/experiments/test_remote.py
@@ -77,8 +77,9 @@ def test_push_diverged(tmp_dir, scm, dvc, git_upstream, exp_stage):
     git_upstream.tmp_dir.scm.set_ref(str(ref_info), remote_rev)
 
     assert dvc.experiments.push(git_upstream.remote, [ref_info.name]) == {
-        "pushed": [],
+        "diverged": [ref_info.name],
         "url": None,
+        "uploaded": 0,
     }
     assert git_upstream.tmp_dir.scm.get_ref(str(ref_info)) == remote_rev
 


Code review comments:
diff_hunk: @@ -18,34 +19,60 @@ def raise_error_if_all_disabled(self):
                 "`--rev` or `--all-commits` flag."
             )
 
-    def run(self):
+    @staticmethod
+    def log_result(result: Dict[str, Any], remote: str):
         from dvc.utils import humanize
 
-        self.raise_error_if_all_disabled()
+        def join_exps(exps):
+            return humanize.join([f"[bold]{e}[/]" for e in exps])
 
-        result = self.repo.experiments.push(
-            self.args.git_remote,
-            self.args.experiment,
-            all_commits=self.args.all_commits,
-            rev=self.args.rev,
-            num=self.args.num,
-            force=self.args.force,
-            push_cache=self.args.push_cache,
-            dvc_remote=self.args.dvc_remote,
-            jobs=self.args.jobs,
-            run_cache=self.args.run_cache,
-        )
-        if pushed_exps := result.get("pushed"):
-            ui.write(
-                f"Pushed experiment {humanize.join(map(repr, pushed_exps))} "
-                f"to Git remote {self.args.git_remote!r}."
+        if diverged_exps := result.get("diverged"):
+            exps = join_exps(diverged_exps)
+            ui.warn(
+                f"Local experiment {exps} has diverged "
+                "from remote experiment with the same name. "
+                "To override the remote experiment re-run with '--force'."
             )
-        else:
+        if uptodate_exps := result.get("up_to_date"):
+            verb = "are" if len(uptodate_exps) > 1 else "is"
+            exps = join_exps(uptodate_exps)
+            ui.write(f"Experiment {exps} {verb} up to date.", styled=True)
+        if pushed_exps := result.get("success"):
+            exps = join_exps(pushed_exps)
+            ui.write(f"Pushed experiment {exps} to Git remote {remote!r}.", styled=True)
+        if not uptodate_exps and not pushed_exps:
             ui.write("No experiments to push.")
 
         if project_url := result.get("url"):
             ui.write("[yellow]View your experiments at", project_url, styled=True)
path: dvc/commands/experiments/push.py
text: @user1:
Not a blocker if it's hard to get the URL in this scenario, but could we show it even when nothing new gets pushed? If I was trying to push an exp that was pushed already, I still may want to see it in Studio or see which experiments are there already.

@author:
We did this so that we don’t spam Studio too much. Maybe we should pretend as if we pushed something here, lie to Studio.

diff_hunk: @@ -18,34 +19,60 @@ def raise_error_if_all_disabled(self):
                 "`--rev` or `--all-commits` flag."
             )
 
-    def run(self):
+    @staticmethod
+    def log_result(result: Dict[str, Any], remote: str):
         from dvc.utils import humanize
 
-        self.raise_error_if_all_disabled()
+        def join_exps(exps):
+            return humanize.join([f"[bold]{e}[/]" for e in exps])
 
-        result = self.repo.experiments.push(
-            self.args.git_remote,
-            self.args.experiment,
-            all_commits=self.args.all_commits,
-            rev=self.args.rev,
-            num=self.args.num,
-            force=self.args.force,
-            push_cache=self.args.push_cache,
-            dvc_remote=self.args.dvc_remote,
-            jobs=self.args.jobs,
-            run_cache=self.args.run_cache,
-        )
-        if pushed_exps := result.get("pushed"):
-            ui.write(
-                f"Pushed experiment {humanize.join(map(repr, pushed_exps))} "
-                f"to Git remote {self.args.git_remote!r}."
+        if diverged_exps := result.get("diverged"):
+            exps = join_exps(diverged_exps)
+            ui.warn(
+                f"Local experiment {exps} has diverged "
+                "from remote experiment with the same name. "
+                "To override the remote experiment re-run with '--force'."
             )
-        else:
+        if uptodate_exps := result.get("up_to_date"):
+            verb = "are" if len(uptodate_exps) > 1 else "is"
+            exps = join_exps(uptodate_exps)
+            ui.write(f"Experiment {exps} {verb} up to date.", styled=True)
path: dvc/commands/experiments/push.py
text: @author:
Sometimes the first letter gets pushed to the end of the line, investigating why it's happening, something to do with Tqdm. :(

@user1:
Haven't noticed it yet FWIW. 

Can we make the message `Experiment {exps} {verb} up to date on Git remote {remote}`? It would make clear we are talking about Git being up to date rather than the DVC cache, and it would be more similar to the message below for pushed experiments.