From 77b937b0015108cbc4fdb7a6f551c1aa77a52400 Mon Sep 17 00:00:00 2001 From: Alexander Schepanovski Date: Fri, 3 Jan 2020 19:05:54 +0700 Subject: [PATCH 1/2] dvc: drop unused params in Repo.brancher() --- dvc/repo/brancher.py | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/dvc/repo/brancher.py b/dvc/repo/brancher.py index 7213cb37a..c1ebff3cb 100644 --- a/dvc/repo/brancher.py +++ b/dvc/repo/brancher.py @@ -5,12 +5,7 @@ from dvc.scm.tree import WorkingTree def brancher( # noqa: E302 - self, - branches=None, - all_branches=False, - tags=None, - all_tags=False, - all_commits=False, + self, all_branches=False, all_tags=False, all_commits=False ): """Generator that iterates over specified revisions. @@ -26,7 +21,7 @@ def brancher( # noqa: E302 - empty string it there is no branches to iterate over - "Working Tree" if there are uncommitted changes in the SCM repo """ - if not any([branches, all_branches, tags, all_tags, all_commits]): + if not any([all_branches, all_tags, all_commits]): yield "" return @@ -42,16 +37,10 @@ def brancher( # noqa: E302 revs = scm.list_all_commits() else: if all_branches: - branches = scm.list_branches() + revs.extend(scm.list_branches()) if all_tags: - tags = scm.list_tags() - - if branches is not None: - revs.extend(branches) - - if tags is not None: - revs.extend(tags) + revs.extend(scm.list_tags()) # NOTE: it might be a good idea to wrap this loop in try/finally block # to don't leave the tree on some unexpected branch after the From ecd511fc694d0cd8ec144c22064388e7a225d9a3 Mon Sep 17 00:00:00 2001 From: Alexander Schepanovski Date: Fri, 3 Jan 2020 19:09:34 +0700 Subject: [PATCH 2/2] dvc: safely return to saved tree in brancher --- dvc/repo/brancher.py | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/dvc/repo/brancher.py b/dvc/repo/brancher.py index c1ebff3cb..ca6dfe84f 100644 --- a/dvc/repo/brancher.py +++ b/dvc/repo/brancher.py @@ -42,13 +42,9 @@ def brancher( # noqa: E302 if all_tags: revs.extend(scm.list_tags()) - # NOTE: it might be a good idea to wrap this loop in try/finally block - # to don't leave the tree on some unexpected branch after the - # `brancher()`, but this could cause problems on exception handling - # code which might expect the tree on which exception was raised to - # stay in place. This behavior is a subject to change. - for sha, names in group_by(scm.resolve_rev, revs).items(): - self.tree = CleanTree(scm.get_tree(sha)) - yield ", ".join(names) - - self.tree = saved_tree + try: + for sha, names in group_by(scm.resolve_rev, revs).items(): + self.tree = CleanTree(scm.get_tree(sha)) + yield ", ".join(names) + finally: + self.tree = saved_tree