From 7ccf8e21cd147332db14c0e96cc60a0e2bfa2e46 Mon Sep 17 00:00:00 2001 From: karajan1001 Date: Wed, 9 Feb 2022 11:09:01 +0800 Subject: [PATCH] exp show: show exp when checkpoint is running(#7329) fix: #7329 In a previous PR, we introduce a bug which use `HEAD` as the workspace commit, while if there is a checkpoint is running, the `HEAD` is actually changing all the time, we should use `refs/exps/exec/EXEC_BASELINE` instead. 1. add `fix_exp_head` transfer in `dvc/scm.py::resolve_rev` 2. remove fix_exp_head from `diff`, `metrics diff`, `params diff` and `plots diff` 3. modify a functional test to simulate the running conditions --- dvc/repo/diff.py | 4 +--- dvc/repo/experiments/diff.py | 7 +------ dvc/repo/metrics/diff.py | 4 +--- dvc/repo/params/diff.py | 4 +--- dvc/repo/plots/diff.py | 5 +---- dvc/scm.py | 8 ++++---- tests/func/experiments/test_show.py | 4 +++- 7 files changed, 12 insertions(+), 24 deletions(-) diff --git a/dvc/repo/diff.py b/dvc/repo/diff.py index aefda984d8..7b4761a59c 100644 --- a/dvc/repo/diff.py +++ b/dvc/repo/diff.py @@ -5,7 +5,6 @@ from dvc.exceptions import PathMissingError from dvc.repo import locked -from dvc.repo.experiments.utils import fix_exp_head logger = logging.getLogger(__name__) @@ -27,8 +26,7 @@ def diff(self, a_rev="HEAD", b_rev=None, targets=None): repo_fs = RepoFileSystem(self) - a_rev = fix_exp_head(self.scm, a_rev) - b_rev = fix_exp_head(self.scm, b_rev) if b_rev else "workspace" + b_rev = b_rev if b_rev else "workspace" results = {} missing_targets = {} for rev in self.brancher(revs=[a_rev, b_rev]): diff --git a/dvc/repo/experiments/diff.py b/dvc/repo/experiments/diff.py index 66c4584e0b..7ad7e7b02b 100644 --- a/dvc/repo/experiments/diff.py +++ b/dvc/repo/experiments/diff.py @@ -1,6 +1,5 @@ import logging -from dvc.repo.experiments.utils import fix_exp_head from dvc.utils.diff import diff as _diff from dvc.utils.diff import format_dict @@ -15,16 +14,12 @@ def diff(repo, *args, a_rev=None, b_rev=None, param_deps=False, **kwargs): return {} if a_rev: - a_rev = fix_exp_head(repo.scm, a_rev) rev = resolve_rev(repo.scm, a_rev) old = _collect_experiment_commit(repo, rev, param_deps=param_deps) else: - old = _collect_experiment_commit( - repo, fix_exp_head(repo.scm, "HEAD"), param_deps=param_deps - ) + old = _collect_experiment_commit(repo, "HEAD", param_deps=param_deps) if b_rev: - b_rev = fix_exp_head(repo.scm, b_rev) rev = resolve_rev(repo.scm, b_rev) new = _collect_experiment_commit(repo, rev, param_deps=param_deps) else: diff --git a/dvc/repo/metrics/diff.py b/dvc/repo/metrics/diff.py index b76ed6732b..0f0a2550ae 100644 --- a/dvc/repo/metrics/diff.py +++ b/dvc/repo/metrics/diff.py @@ -1,4 +1,3 @@ -from dvc.repo.experiments.utils import fix_exp_head from dvc.utils.diff import diff as _diff from dvc.utils.diff import format_dict @@ -10,8 +9,7 @@ def diff(repo, *args, a_rev=None, b_rev=None, **kwargs): with_unchanged = kwargs.pop("all", False) a_rev = a_rev or "HEAD" - a_rev = fix_exp_head(repo.scm, a_rev) - b_rev = fix_exp_head(repo.scm, b_rev) or "workspace" + b_rev = b_rev or "workspace" metrics = repo.metrics.show(*args, **kwargs, revs=[a_rev, b_rev]) old = metrics.get(a_rev, {}).get("data", {}) diff --git a/dvc/repo/params/diff.py b/dvc/repo/params/diff.py index f08255b3fe..15688852db 100644 --- a/dvc/repo/params/diff.py +++ b/dvc/repo/params/diff.py @@ -1,4 +1,3 @@ -from dvc.repo.experiments.utils import fix_exp_head from dvc.utils.diff import diff as _diff from dvc.utils.diff import format_dict @@ -10,8 +9,7 @@ def diff(repo, *args, a_rev=None, b_rev=None, **kwargs): with_unchanged = kwargs.pop("all", False) a_rev = a_rev or "HEAD" - a_rev = fix_exp_head(repo.scm, a_rev) - b_rev = fix_exp_head(repo.scm, b_rev) or "workspace" + b_rev = b_rev or "workspace" params = repo.params.show(*args, **kwargs, revs=[a_rev, b_rev]) diff --git a/dvc/repo/plots/diff.py b/dvc/repo/plots/diff.py index 42baebd2f5..73dfabcfb7 100644 --- a/dvc/repo/plots/diff.py +++ b/dvc/repo/plots/diff.py @@ -1,6 +1,3 @@ -from dvc.repo.experiments.utils import fix_exp_head - - def _revisions(repo, revs, experiment): revisions = revs or [] if experiment and len(revisions) == 1: @@ -9,7 +6,7 @@ def _revisions(repo, revs, experiment): revisions.append(baseline[:7]) if len(revisions) <= 1: if len(revisions) == 0 and repo.scm.is_dirty(): - revisions.append(fix_exp_head(repo.scm, "HEAD")) + revisions.append("HEAD") revisions.append("workspace") return revisions diff --git a/dvc/scm.py b/dvc/scm.py index 0873bfbc55..f2e09e0489 100644 --- a/dvc/scm.py +++ b/dvc/scm.py @@ -111,8 +111,10 @@ def clone(url: str, to_path: str, **kwargs): def resolve_rev(scm: "Git", rev: str) -> str: from scmrepo.exceptions import RevError as InternalRevError + from dvc.repo.experiments.utils import fix_exp_head + try: - return scm.resolve_rev(rev) + return scm.resolve_rev(fix_exp_head(scm, rev)) except InternalRevError as exc: # `scm` will only resolve git branch and tag names, # if rev is not a sha it may be an abbreviated experiment name @@ -141,7 +143,6 @@ def iter_revs( all_commits: bool = False, all_experiments: bool = False, ) -> Mapping[str, List[str]]: - from dvc.repo.experiments.utils import fix_exp_head if num < 1 and num != -1: raise InvalidArgumentError(f"Invalid number of commits '{num}'") @@ -160,8 +161,7 @@ def iter_revs( if num == n: break try: - head = fix_exp_head(scm, f"{rev}~{n}") - assert head + head = f"{rev}~{n}" revs.append(resolve_rev(scm, head)) except RevError: break diff --git a/tests/func/experiments/test_show.py b/tests/func/experiments/test_show.py index 7102ddc98a..8e0112671a 100644 --- a/tests/func/experiments/test_show.py +++ b/tests/func/experiments/test_show.py @@ -388,7 +388,7 @@ def test_show_running_executor(tmp_dir, scm, dvc, exp_stage): def test_show_running_checkpoint( tmp_dir, scm, dvc, checkpoint_stage, workspace, mocker ): - from dvc.repo.experiments.base import EXEC_BRANCH + from dvc.repo.experiments.base import EXEC_BASELINE, EXEC_BRANCH from dvc.repo.experiments.executor.local import TempDirExecutor baseline_rev = scm.get_rev() @@ -422,6 +422,8 @@ def test_show_running_checkpoint( ) if workspace: scm.set_ref(EXEC_BRANCH, str(exp_ref), symbolic=True) + scm.set_ref(EXEC_BASELINE, str(baseline_rev)) + scm.checkout(str(exp_ref)) results = dvc.experiments.show()