Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update: --recursive flag #3760

Merged
merged 4 commits into from
May 8, 2020
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
update: --recursive flag
nik123 committed May 7, 2020

Verified

This commit was signed with the committer’s verified signature.
reneme René Meusel
commit 381726589e8b3d5e01eff37f72a582056c8061db
22 changes: 16 additions & 6 deletions dvc/command/update.py
Original file line number Diff line number Diff line change
@@ -10,12 +10,15 @@
class CmdUpdate(CmdBase):
def run(self):
ret = 0
for target in self.args.targets:
try:
self.repo.update(target, self.args.rev)
except DvcException:
logger.exception("failed to update '{}'.".format(target))
ret = 1
try:
self.repo.update(
targets=self.args.targets,
rev=self.args.rev,
recursive=self.args.recursive,
)
except DvcException:
logger.exception("failed update data")
ret = 1
return ret


@@ -37,4 +40,11 @@ def add_parser(subparsers, parent_parser):
help="Git revision (e.g. SHA, branch, tag)",
metavar="<commit>",
)
update_parser.add_argument(
"-R",
"--recursive",
action="store_true",
default=False,
help="Update all stages in the specified directory.",
)
update_parser.set_defaults(func=CmdUpdate)
22 changes: 14 additions & 8 deletions dvc/repo/update.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
from . import locked
from ..dvcfile import Dvcfile


@locked
def update(self, target, rev=None):
from ..dvcfile import Dvcfile
def update(self, targets=None, rev=None, recursive=False):
if not targets:
stages = self.collect(targets, recursive=recursive)
else:
stages = set()
for target in targets:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could also

if isinstance(targets, str):
    targets = [targets]     

just as a convenience (we have that in dvc add too), since it is guaranteed someone will try to misuse it 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way are the any other commands which should contain isinstance(targets, str) check? repo.used_cache, repo.status, repo._checkout seem good candidates.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nik123 Probably not used_cache but the rest could use it, sure. But let's not bother with it in this PR, it is out of scope 🙂

stages.update(self.collect(target, recursive=recursive))

dvcfile = Dvcfile(self, target)
stage = dvcfile.stage
stage.update(rev)
for stage in stages:
stage.update(rev)
dvcfile = Dvcfile(self, stage.path)
dvcfile.dump(stage)
stages.add(stage)

dvcfile.dump(stage)

return stage
return list(stages)
4 changes: 2 additions & 2 deletions tests/func/test_repro.py
Original file line number Diff line number Diff line change
@@ -956,12 +956,12 @@ def test(self, mock_prompt):

self.assertNotEqual(self.dvc.status(), {})

self.dvc.update(import_stage.path)
self.dvc.update([import_stage.path])
self.assertTrue(os.path.exists("import"))
self.assertTrue(filecmp.cmp("import", self.BAR, shallow=False))
self.assertEqual(self.dvc.status([import_stage.path]), {})

self.dvc.update(import_remote_stage.path)
self.dvc.update([import_remote_stage.path])
self.assertEqual(self.dvc.status([import_remote_stage.path]), {})

stages = self.dvc.reproduce(cmd_stage.addressing)
69 changes: 63 additions & 6 deletions tests/func/test_update.py
Original file line number Diff line number Diff line change
@@ -26,7 +26,7 @@ def test_update_import(tmp_dir, dvc, erepo_dir, cached):

assert old_rev != new_rev

dvc.update(stage.path)
dvc.update([stage.path])
assert (tmp_dir / "version").read_text() == "updated"

stage = Dvcfile(dvc, stage.path).stage
@@ -65,7 +65,7 @@ def test_update_import_after_remote_updates_to_dvc(tmp_dir, dvc, erepo_dir):
assert changed_dep[0].startswith("version ")
assert changed_dep[1] == "update available"

dvc.update(stage.path)
dvc.update([stage.path])

assert dvc.status([stage.path]) == {}

@@ -106,7 +106,7 @@ def test_update_before_and_after_dvc_init(tmp_dir, dvc, git_dir):
]
}

dvc.update(stage.path)
dvc.update([stage.path])

assert (tmp_dir / "file").read_text() == "second version"
assert dvc.status([stage.path]) == {}
@@ -127,7 +127,7 @@ def test_update_import_url(tmp_dir, dvc, tmp_path_factory):
src.write_text("updated file content")

assert dvc.status([stage.path]) == {}
dvc.update(stage.path)
dvc.update([stage.path])
assert dvc.status([stage.path]) == {}

assert dst.is_file()
@@ -149,7 +149,7 @@ def test_update_rev(tmp_dir, dvc, scm, git_dir):
git_dir.scm_gen({"foo": "foobar foo"}, commit="branch2 commit")
branch2_head = git_dir.scm.get_rev()

stage = dvc.update("foo.dvc", rev="branch1")
stage = dvc.update(["foo.dvc"], rev="branch1")[0]
assert stage.deps[0].def_repo == {
"url": fspath(git_dir),
"rev": "branch1",
@@ -158,11 +158,68 @@ def test_update_rev(tmp_dir, dvc, scm, git_dir):
with open(fspath_py35(tmp_dir / "foo")) as f:
assert "foobar" == f.read()

stage = dvc.update("foo.dvc", rev="branch2")
stage = dvc.update(["foo.dvc"], rev="branch2")[0]
assert stage.deps[0].def_repo == {
"url": fspath(git_dir),
"rev": "branch2",
"rev_lock": branch2_head,
}
with open(fspath_py35(tmp_dir / "foo")) as f:
assert "foobar foo" == f.read()


def test_update_recursive(tmp_dir, dvc, erepo_dir):
with erepo_dir.branch("branch", new=True), erepo_dir.chdir():
erepo_dir.scm_gen(
{"foo1": "text1", "foo2": "text2", "foo3": "text3"},
commit="add foo files",
)
old_rev = erepo_dir.scm.get_rev()

tmp_dir.gen({"dir": {"subdir": {}}})
stage1 = dvc.imp(
fspath(erepo_dir), "foo1", os.path.join("dir", "foo1"), rev="branch",
)
stage2 = dvc.imp(
fspath(erepo_dir),
"foo2",
os.path.join("dir", "subdir", "foo2"),
rev="branch",
)
stage3 = dvc.imp(
fspath(erepo_dir),
"foo3",
os.path.join("dir", "subdir", "foo3"),
rev="branch",
)

assert (tmp_dir / os.path.join("dir", "foo1")).read_text() == "text1"
assert (
tmp_dir / os.path.join("dir", "subdir", "foo2")
).read_text() == "text2"
assert (
tmp_dir / os.path.join("dir", "subdir", "foo3")
).read_text() == "text3"

assert stage1.deps[0].def_repo["rev_lock"] == old_rev
assert stage2.deps[0].def_repo["rev_lock"] == old_rev
assert stage3.deps[0].def_repo["rev_lock"] == old_rev

with erepo_dir.branch("branch", new=False), erepo_dir.chdir():
erepo_dir.scm_gen(
{"foo1": "updated1", "foo2": "updated2", "foo3": "updated3"},
"",
"update foo content",
)
new_rev = erepo_dir.scm.get_rev()

assert old_rev != new_rev

dvc.update(["dir"], recursive=True)

stage1 = Dvcfile(dvc, stage1.path).stage
stage2 = Dvcfile(dvc, stage2.path).stage
stage3 = Dvcfile(dvc, stage3.path).stage
assert stage1.deps[0].def_repo["rev_lock"] == new_rev
assert stage2.deps[0].def_repo["rev_lock"] == new_rev
assert stage3.deps[0].def_repo["rev_lock"] == new_rev
17 changes: 7 additions & 10 deletions tests/unit/command/test_update.py
Original file line number Diff line number Diff line change
@@ -1,20 +1,17 @@
import pytest

from dvc.cli import parse_args
from dvc.command.update import CmdUpdate


@pytest.mark.parametrize(
"command,rev", [(["update"], None), (["update", "--rev", "REV"], "REV")]
)
def test_update(dvc, mocker, command, rev):
targets = ["target1", "target2", "target3"]
cli_args = parse_args(command + targets)
def test_update(dvc, mocker):
cli_args = parse_args(
["update", "target1", "target2", "--rev", "REV", "--recursive"]
)
assert cli_args.func == CmdUpdate
cmd = cli_args.func(cli_args)
m = mocker.patch("dvc.repo.Repo.update")

assert cmd.run() == 0

calls = [mocker.call(target, rev) for target in targets]
m.assert_has_calls(calls)
m.assert_called_once_with(
targets=["target1", "target2"], rev="REV", recursive=True,
)