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

run: use -f/--force to overwrite, fix msg #4011

Merged
merged 1 commit into from
Jun 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
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
5 changes: 3 additions & 2 deletions dvc/command/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def run(self):
fname=self.args.file,
wdir=self.args.wdir,
no_exec=self.args.no_exec,
overwrite=self.args.overwrite,
force=self.args.force,
run_cache=not self.args.no_run_cache,
no_commit=self.args.no_commit,
outs_persist=self.args.outs_persist,
Expand Down Expand Up @@ -176,7 +176,8 @@ def add_parser(subparsers, parent_parser):
help="Only create stage file without actually running it.",
)
run_parser.add_argument(
"--overwrite",
"-f",
"--force",
action="store_true",
default=False,
help="Overwrite existing stage",
Expand Down
18 changes: 0 additions & 18 deletions dvc/dvcfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,9 @@

from voluptuous import MultipleInvalid

import dvc.prompt as prompt
from dvc.exceptions import DvcException
from dvc.stage import serialize
from dvc.stage.exceptions import (
StageFileAlreadyExistsError,
StageFileBadNameError,
StageFileDoesNotExistError,
StageFileFormatError,
Expand Down Expand Up @@ -109,9 +107,6 @@ def validate(cls, d, fname=None):
except MultipleInvalid as exc:
raise StageFileFormatError(f"'{fname}' format error: {exc}")

def remove_with_prompt(self, force=False):
raise NotImplementedError

def remove(self, force=False):
with contextlib.suppress(FileNotFoundError):
os.unlink(self.path)
Expand Down Expand Up @@ -148,19 +143,6 @@ def dump(self, stage, **kwargs):
dump_yaml(self.path, serialize.to_single_stage_file(stage))
self.repo.scm.track_file(self.relpath)

def remove_with_prompt(self, force=False):
Copy link
Member Author

Choose a reason for hiding this comment

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

This was only being used as force=True making it equivalent to .remove().

if not self.exists():
return

msg = (
"'{}' already exists. Do you wish to run the command and "
"overwrite it?".format(relpath(self.path))
)
if not (force or prompt.confirm(msg)):
raise StageFileAlreadyExistsError(self.path)

self.remove()

def remove_stage(self, stage):
self.remove()

Expand Down
2 changes: 1 addition & 1 deletion dvc/repo/add.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ def _create_stages(repo, targets, fname, pbar=None, external=False):
external=external,
)
if stage:
Dvcfile(repo, stage.path).remove_with_prompt(force=True)
Dvcfile(repo, stage.path).remove()
skshetry marked this conversation as resolved.
Show resolved Hide resolved

repo._reset()

Expand Down
2 changes: 1 addition & 1 deletion dvc/repo/imp_url.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def imp_url(self, url, out=None, fname=None, erepo=None, frozen=True):
return None

dvcfile = Dvcfile(self, stage.path)
dvcfile.remove_with_prompt(force=True)
dvcfile.remove()

try:
self.check_modified_graph([stage])
Expand Down
29 changes: 21 additions & 8 deletions dvc/repo/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from funcy import concat, first, lfilter

from dvc.exceptions import InvalidArgumentError
from dvc.stage import PipelineStage
from dvc.stage.exceptions import (
DuplicateStageName,
InvalidStageName,
Expand Down Expand Up @@ -56,10 +57,25 @@ def _get_file_path(kwargs):
)


def _check_stage_exists(dvcfile, stage):
if not dvcfile.exists():
return

hint = "Use '--force' to overwrite."
if stage.__class__ != PipelineStage:
raise StageFileAlreadyExistsError(
f"'{stage.relpath}' already exists. {hint}"
)
elif stage.name and stage.name in dvcfile.stages:
raise DuplicateStageName(
f"Stage '{stage.name}' already exists in '{stage.relpath}'. {hint}"
)


@locked
@scm_context
def run(self, fname=None, no_exec=False, single_stage=False, **kwargs):
from dvc.stage import PipelineStage, Stage, create_stage
from dvc.stage import Stage, create_stage
from dvc.dvcfile import Dvcfile, PIPELINE_FILE

if not kwargs.get("cmd"):
Expand Down Expand Up @@ -93,13 +109,10 @@ def run(self, fname=None, no_exec=False, single_stage=False, **kwargs):
return None

dvcfile = Dvcfile(self, stage.path)
if dvcfile.exists():
if kwargs.get("overwrite", True):
dvcfile.remove_stage(stage)
elif stage_cls != PipelineStage:
raise StageFileAlreadyExistsError(dvcfile.relpath)
elif stage_name and stage_name in dvcfile.stages:
raise DuplicateStageName(stage_name, dvcfile)
if kwargs.get("force", True):
dvcfile.remove_stage(stage)
else:
_check_stage_exists(dvcfile, stage)

try:
self.check_modified_graph([stage])
Expand Down
11 changes: 2 additions & 9 deletions dvc/stage/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,7 @@ def __init__(self, fname):


class StageFileAlreadyExistsError(DvcException):
def __init__(self, relpath):
msg = f"not overwriting '{relpath}'"
super().__init__(msg)
pass


class StageFileIsNotDvcFileError(DvcException):
Expand Down Expand Up @@ -100,12 +98,7 @@ def __init__(self, file):


class DuplicateStageName(DvcException):
def __init__(self, name, file):
super().__init__(
"Stage '{name}' already exists in '{relpath}'.".format(
name=name, relpath=file.relpath
)
)
pass


class InvalidStageName(DvcException):
Expand Down
4 changes: 2 additions & 2 deletions tests/func/test_run_multistage.py
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,8 @@ def test_run_already_exists(tmp_dir, dvc, run_copy):
tmp_dir.dvc_gen("foo", "foo")
run_copy("foo", "bar", name="copy")
with pytest.raises(DuplicateStageName):
run_copy("bar", "foobar", name="copy", overwrite=False)
run_copy("bar", "foobar", name="copy", overwrite=True)
run_copy("bar", "foobar", name="copy", force=False)
run_copy("bar", "foobar", name="copy", force=True)


supported_params = {
Expand Down
28 changes: 14 additions & 14 deletions tests/func/test_run_single_stage.py
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ def test(self):
ret = main(
[
"run",
"--overwrite",
"--force",
"--no-run-cache",
"--single-stage",
"-d",
Expand Down Expand Up @@ -408,7 +408,7 @@ def test(self):
ret = main(
[
"run",
"--overwrite",
"--force",
"--no-run-cache",
"--single-stage",
"-d",
Expand Down Expand Up @@ -467,7 +467,7 @@ def test(self):
ret = main(
[
"run",
"--overwrite",
"--force",
"--no-run-cache",
"--single-stage",
"-d",
Expand Down Expand Up @@ -552,7 +552,7 @@ def test(self):
self.FOO,
"-d",
self.CODE,
"--overwrite",
"--force",
"--no-run-cache",
"--single-stage",
"-o",
Expand All @@ -576,7 +576,7 @@ def test(self):
ret = main(
[
"run",
"--overwrite",
"--force",
"--single-stage",
"--file",
"out.dvc",
Expand Down Expand Up @@ -676,19 +676,19 @@ def test_rerun_deterministic_ignore_cache(tmp_dir, run_copy):


def test_rerun_callback(dvc):
def run_callback(overwrite=False):
def run_callback(force=False):
return dvc.run(
cmd="echo content > out",
outs=["out"],
deps=[],
overwrite=overwrite,
force=force,
single_stage=True,
)

assert run_callback() is not None
with pytest.raises(StageFileAlreadyExistsError):
assert run_callback() is not None
assert run_callback(overwrite=True) is not None
assert run_callback(force=True) is not None


def test_rerun_changed_dep(tmp_dir, run_copy):
Expand All @@ -697,8 +697,8 @@ def test_rerun_changed_dep(tmp_dir, run_copy):

tmp_dir.gen("foo", "changed content")
with pytest.raises(StageFileAlreadyExistsError):
run_copy("foo", "out", overwrite=False, single_stage=True)
assert run_copy("foo", "out", overwrite=True, single_stage=True)
run_copy("foo", "out", force=False, single_stage=True)
assert run_copy("foo", "out", force=True, single_stage=True)


def test_rerun_changed_stage(tmp_dir, run_copy):
Expand All @@ -707,7 +707,7 @@ def test_rerun_changed_stage(tmp_dir, run_copy):

tmp_dir.gen("bar", "bar content")
with pytest.raises(StageFileAlreadyExistsError):
run_copy("bar", "out", overwrite=False, single_stage=True)
run_copy("bar", "out", force=False, single_stage=True)


def test_rerun_changed_out(tmp_dir, run_copy):
Expand All @@ -716,7 +716,7 @@ def test_rerun_changed_out(tmp_dir, run_copy):

Path("out").write_text("modification")
with pytest.raises(StageFileAlreadyExistsError):
run_copy("foo", "out", overwrite=False, single_stage=True)
run_copy("foo", "out", force=False, single_stage=True)


class TestRunCommit(TestDvc):
Expand Down Expand Up @@ -870,7 +870,7 @@ def _run_twice_with_same_outputs(self):
"run",
self._outs_command,
self.FOO,
"--overwrite",
"--force",
"--single-stage",
f"echo {self.BAR_CONTENTS} >> {self.FOO}",
]
Expand Down Expand Up @@ -946,7 +946,7 @@ def test_ignore_run_cache(self):

cmd = [
"run",
"--overwrite",
"--force",
"--single-stage",
"--deps",
"immutable",
Expand Down
8 changes: 4 additions & 4 deletions tests/unit/command/test_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def test_run(mocker, dvc):
"--wdir",
"wdir",
"--no-exec",
"--overwrite",
"--force",
"--no-run-cache",
"--no-commit",
"--outs-persist",
Expand Down Expand Up @@ -64,7 +64,7 @@ def test_run(mocker, dvc):
fname="file",
wdir="wdir",
no_exec=True,
overwrite=True,
force=True,
run_cache=False,
no_commit=True,
always_changed=True,
Expand Down Expand Up @@ -94,7 +94,7 @@ def test_run_args_from_cli(mocker, dvc):
fname=None,
wdir=None,
no_exec=False,
overwrite=False,
force=False,
run_cache=True,
no_commit=False,
always_changed=False,
Expand Down Expand Up @@ -124,7 +124,7 @@ def test_run_args_with_spaces(mocker, dvc):
fname=None,
wdir=None,
no_exec=False,
overwrite=False,
force=False,
run_cache=True,
no_commit=False,
always_changed=False,
Expand Down