From 26c471eb1acb7f390d6dd234bee86c39169b1d37 Mon Sep 17 00:00:00 2001 From: dberenbaum Date: Fri, 1 Dec 2023 13:45:03 -0500 Subject: [PATCH 1/5] exp: ignore workspace errors during push/pull --- dvc/repo/fetch.py | 4 +++- tests/func/experiments/test_remote.py | 15 +++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/dvc/repo/fetch.py b/dvc/repo/fetch.py index 5bdfc9acdb..041235401b 100644 --- a/dvc/repo/fetch.py +++ b/dvc/repo/fetch.py @@ -22,7 +22,7 @@ def _onerror(entry, exc): return _onerror -def _collect_indexes( # noqa: PLR0913 +def _collect_indexes( # noqa: PLR0913,C901 repo, targets=None, remote=None, @@ -80,6 +80,8 @@ def outs_filter(out: "Output") -> bool: indexes[rev or "workspace"] = idx except Exception as exc: + if rev == "workspace" and rev not in revs: + continue if onerror: onerror(rev, None, exc) collection_exc = exc diff --git a/tests/func/experiments/test_remote.py b/tests/func/experiments/test_remote.py index 1fe9d80933..60b3665cb7 100644 --- a/tests/func/experiments/test_remote.py +++ b/tests/func/experiments/test_remote.py @@ -1,3 +1,5 @@ +import logging + import pytest from funcy import first @@ -360,3 +362,16 @@ def test_get(tmp_dir, scm, dvc, exp_stage, erepo_dir, use_ref): rev=exp_ref.name if use_ref else exp_rev, ) assert (erepo_dir / "params.yaml").read_text().strip() == "foo: 2" + + +def test_push_invalid_workspace( + tmp_dir, scm, dvc, git_upstream, exp_stage, local_remote, caplog +): + dvc.experiments.run() + + with open("dvc.yaml", mode="a") as f: + f.write("\ninvalid") + + with caplog.at_level(logging.ERROR, logger="dvc"): + dvc.experiments.push(git_upstream.remote, push_cache=True) + assert "failed to collect" not in caplog.text From 4a068ba47bb5e8f5e8c3fef8bf310b6bd9ef4fcf Mon Sep 17 00:00:00 2001 From: dberenbaum Date: Mon, 4 Dec 2023 08:46:51 -0500 Subject: [PATCH 2/5] refactor fetch revs warning --- dvc/repo/fetch.py | 6 +++--- tests/func/experiments/test_remote.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dvc/repo/fetch.py b/dvc/repo/fetch.py index 041235401b..4b2cfaaa00 100644 --- a/dvc/repo/fetch.py +++ b/dvc/repo/fetch.py @@ -79,13 +79,13 @@ def outs_filter(out: "Output") -> bool: idx.data["repo"].onerror = _make_index_onerror(onerror, rev) indexes[rev or "workspace"] = idx - except Exception as exc: - if rev == "workspace" and rev not in revs: + except Exception as exc: # noqa: BLE001 + if revs and rev == "workspace" and rev not in revs: continue if onerror: onerror(rev, None, exc) collection_exc = exc - logger.exception("failed to collect '%s'", rev or "workspace") + logger.warning("failed to collect '%s'", rev or "workspace") if not indexes and collection_exc: raise collection_exc diff --git a/tests/func/experiments/test_remote.py b/tests/func/experiments/test_remote.py index 60b3665cb7..87b28a2713 100644 --- a/tests/func/experiments/test_remote.py +++ b/tests/func/experiments/test_remote.py @@ -372,6 +372,6 @@ def test_push_invalid_workspace( with open("dvc.yaml", mode="a") as f: f.write("\ninvalid") - with caplog.at_level(logging.ERROR, logger="dvc"): + with caplog.at_level(logging.WARNING, logger="dvc"): dvc.experiments.push(git_upstream.remote, push_cache=True) assert "failed to collect" not in caplog.text From 7f33e7522147bc57d93b1edb689cace91234a8d5 Mon Sep 17 00:00:00 2001 From: dberenbaum Date: Mon, 4 Dec 2023 08:52:15 -0500 Subject: [PATCH 3/5] mention failed revs are skipped --- dvc/repo/fetch.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dvc/repo/fetch.py b/dvc/repo/fetch.py index 4b2cfaaa00..37760dc278 100644 --- a/dvc/repo/fetch.py +++ b/dvc/repo/fetch.py @@ -85,7 +85,7 @@ def outs_filter(out: "Output") -> bool: if onerror: onerror(rev, None, exc) collection_exc = exc - logger.warning("failed to collect '%s'", rev or "workspace") + logger.warning("failed to collect '%s', skipping", rev or "workspace") if not indexes and collection_exc: raise collection_exc From 3b521f51f26675ff6a296309b55375bfa6a728af Mon Sep 17 00:00:00 2001 From: dberenbaum Date: Mon, 4 Dec 2023 09:26:17 -0500 Subject: [PATCH 4/5] add workspace arg to brancher --- dvc/repo/brancher.py | 5 ++++- dvc/repo/fetch.py | 5 ++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/dvc/repo/brancher.py b/dvc/repo/brancher.py index 3499cf0f08..38609677de 100644 --- a/dvc/repo/brancher.py +++ b/dvc/repo/brancher.py @@ -20,6 +20,7 @@ def brancher( all_tags=False, all_commits=False, all_experiments=False, + workspace=True, commit_date: Optional[str] = None, sha_only=False, num=1, @@ -31,6 +32,7 @@ def brancher( all_branches (bool): iterate over all available branches. all_commits (bool): iterate over all commits. all_tags (bool): iterate over all available tags. + workspace (bool): include workspace. commit_date (str): Keep experiments from the commits after(include) a certain date. Date must match the extended ISO 8601 format (YYYY-MM-DD). @@ -73,7 +75,8 @@ def brancher( logger.trace("switching fs to workspace") self.fs = LocalFileSystem(url=self.root_dir) - yield "workspace" + if workspace: + yield "workspace" revs = revs.copy() if revs else [] if "workspace" in revs: diff --git a/dvc/repo/fetch.py b/dvc/repo/fetch.py index 37760dc278..5f0d01a885 100644 --- a/dvc/repo/fetch.py +++ b/dvc/repo/fetch.py @@ -22,7 +22,7 @@ def _onerror(entry, exc): return _onerror -def _collect_indexes( # noqa: PLR0913,C901 +def _collect_indexes( # noqa: PLR0913 repo, targets=None, remote=None, @@ -62,6 +62,7 @@ def outs_filter(out: "Output") -> bool: all_branches=all_branches, all_tags=all_tags, all_commits=all_commits, + workspace=False, ): try: repo.config.merge(config) @@ -80,8 +81,6 @@ def outs_filter(out: "Output") -> bool: indexes[rev or "workspace"] = idx except Exception as exc: # noqa: BLE001 - if revs and rev == "workspace" and rev not in revs: - continue if onerror: onerror(rev, None, exc) collection_exc = exc From 185d9bbac73e77d8ac0c1e74708e5b406c97d47e Mon Sep 17 00:00:00 2001 From: dberenbaum Date: Mon, 4 Dec 2023 10:34:11 -0500 Subject: [PATCH 5/5] pass workspace=False only for dvc exp push/pull --- dvc/repo/experiments/pull.py | 4 +++- dvc/repo/experiments/push.py | 4 +++- dvc/repo/fetch.py | 5 ++++- dvc/repo/push.py | 2 ++ tests/func/experiments/test_remote.py | 3 ++- 5 files changed, 14 insertions(+), 4 deletions(-) diff --git a/dvc/repo/experiments/pull.py b/dvc/repo/experiments/pull.py index f643e38cbf..699a54a422 100644 --- a/dvc/repo/experiments/pull.py +++ b/dvc/repo/experiments/pull.py @@ -112,4 +112,6 @@ def _pull_cache( refs = [refs] revs = list(exp_commits(repo.scm, refs)) logger.debug("dvc fetch experiment '%s'", refs) - repo.fetch(jobs=jobs, remote=dvc_remote, run_cache=run_cache, revs=revs) + repo.fetch( + jobs=jobs, remote=dvc_remote, run_cache=run_cache, revs=revs, workspace=False + ) diff --git a/dvc/repo/experiments/push.py b/dvc/repo/experiments/push.py index f62b1b2634..184bf21406 100644 --- a/dvc/repo/experiments/push.py +++ b/dvc/repo/experiments/push.py @@ -188,4 +188,6 @@ def _push_cache( assert isinstance(repo.scm, Git) revs = list(exp_commits(repo.scm, refs)) logger.debug("dvc push experiment '%s'", refs) - return 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, workspace=False + ) diff --git a/dvc/repo/fetch.py b/dvc/repo/fetch.py index 5f0d01a885..3a3c795091 100644 --- a/dvc/repo/fetch.py +++ b/dvc/repo/fetch.py @@ -32,6 +32,7 @@ def _collect_indexes( # noqa: PLR0913 recursive=False, all_commits=False, revs=None, + workspace=True, max_size=None, types=None, config=None, @@ -62,7 +63,7 @@ def outs_filter(out: "Output") -> bool: all_branches=all_branches, all_tags=all_tags, all_commits=all_commits, - workspace=False, + workspace=workspace, ): try: repo.config.merge(config) @@ -105,6 +106,7 @@ def fetch( # noqa: PLR0913 all_commits=False, run_cache=False, revs=None, + workspace=True, max_size=None, types=None, config=None, @@ -149,6 +151,7 @@ def fetch( # noqa: PLR0913 recursive=recursive, all_commits=all_commits, revs=revs, + workspace=workspace, max_size=max_size, types=types, config=config, diff --git a/dvc/repo/push.py b/dvc/repo/push.py index 5a14181577..ae42610d46 100644 --- a/dvc/repo/push.py +++ b/dvc/repo/push.py @@ -46,6 +46,7 @@ def push( # noqa: PLR0913 all_commits=False, run_cache=False, revs=None, + workspace=True, glob=False, ): from fsspec.utils import tokenize @@ -87,6 +88,7 @@ def push( # noqa: PLR0913 recursive=recursive, all_commits=all_commits, revs=revs, + workspace=workspace, push=True, ) diff --git a/tests/func/experiments/test_remote.py b/tests/func/experiments/test_remote.py index 87b28a2713..150279797d 100644 --- a/tests/func/experiments/test_remote.py +++ b/tests/func/experiments/test_remote.py @@ -364,7 +364,7 @@ def test_get(tmp_dir, scm, dvc, exp_stage, erepo_dir, use_ref): assert (erepo_dir / "params.yaml").read_text().strip() == "foo: 2" -def test_push_invalid_workspace( +def test_push_pull_invalid_workspace( tmp_dir, scm, dvc, git_upstream, exp_stage, local_remote, caplog ): dvc.experiments.run() @@ -374,4 +374,5 @@ def test_push_invalid_workspace( with caplog.at_level(logging.WARNING, logger="dvc"): dvc.experiments.push(git_upstream.remote, push_cache=True) + dvc.experiments.pull(git_upstream.remote, pull_cache=True) assert "failed to collect" not in caplog.text