run: raise error when command is not specified#3850
Conversation
skshetry
changed the title
| dvc.run( | ||
| **{ | ||
| "command": "python script.py", | ||
| "cmd": "python script.py {}".format(os.path.join("out", "file")), |
There was a problem hiding this comment.
The command never ran before, and never worked.
| @@ -0,0 +1 @@ | |||
| from tests.func.plots import run_copy_metrics # noqa: F401 | |||
There was a problem hiding this comment.
Created a run_copy_metrics fixture, that'd copy files, mostly used as copying temporarily-dumped file into metrics/plots file. Also, as metrics/params mostly work with tags and commits, run_copy_metrics can set tags and commits itself.
| tmp_dir.gen({"m_temp.yaml": str(val)}) | ||
| run_copy_metrics("m_temp.yaml", "m.yaml", metrics=["m.yaml"]) |
There was a problem hiding this comment.
Similar code exists on metrics/params. A temp file with suffix _temp/_t is created which is copied (to have some cmd and generate metrics/plots as in the real usage).
| *deps, | ||
| "ls {}".format( | ||
| " ".join(dep for i, dep in enumerate(deps) if i % 2) |
There was a problem hiding this comment.
On above code, deps are saved as list with sequence of -d, dep items (flattened out), i just extract every second element of the list.
| last_stage = tmp_dir / "dir" / DVC_FILE | ||
| deps = [os.fspath(origin_copy_2), os.fspath(dir_file_copy)] | ||
| stage = dvc.run( | ||
| cmd="echo {}".format(" ".join(deps)), |
There was a problem hiding this comment.
For these kind of stage that does not generate any outs, I have used echo or ls or cat as a cmd.
| @pytest.mark.parametrize( | ||
| "kwargs", | ||
| [ | ||
| {"outs": ["foo"], "deps": ["bar"]}, | ||
| {"outs": ["foo"], "deps": ["bar"], "name": "copy-foo-bar"}, | ||
| ], | ||
| ) | ||
| def test_run_without_cmd(kwargs): | ||
| with pytest.raises(InvalidArgumentError) as exc: | ||
| Repo().run(**kwargs) | ||
| assert "command is not specified" == str(exc.value) |
There was a problem hiding this comment.
This is the actual test for the code change.
| with self.assertRaises(DependencyDoesNotExistError): | ||
| self.dvc.run( | ||
| cmd="", | ||
| cmd="command", |
There was a problem hiding this comment.
A lot of tests here were cmd is never run (they eventually raise Exception before running the command), I have used command.
| def test_cwd(self): | ||
| self.dvc.run( | ||
| cmd="", deps=[], outs=[self.DATA_DIR], single_stage=True, | ||
| cmd=f"mkdir {self.DATA_DIR}", |
There was a problem hiding this comment.
There should not be too many of the cases were there are no deps but only outs, but in those cases I have used mkdir or touch as cmd.
skshetry
changed the title
|
Noticed that |
@skshetry Oh, probably some env issues. Let's create an issue for it please to investigate further later. |
3 participants
❗ I have followed the Contributing to DVC checklist.
📖 If this PR requires documentation updates, I have created a separate PR (or issue, at least) in dvc.org and linked it here. If the CLI API is changed, I have updated tab completion scripts.
❌ I will check DeepSource, CodeClimate, and other sanity checks below. (We consider them recommendatory and don't expect everything to be addressed. Please fix things that actually improve code or fix bugs.)
Thank you for the contribution - we'll try to review it as soon as possible. 🙏
Previously,
runused to show misleadingMissingOutputErrorwhen no command was specified.