From d1d92a6bbb7a9a1fa674236793c684e213d3c08a Mon Sep 17 00:00:00 2001 From: Saugat Pachhai Date: Wed, 29 Apr 2020 17:49:02 +0545 Subject: [PATCH 1/4] tags: get rid of it --- dvc/cli.py | 2 - dvc/command/pipeline.py | 8 +- dvc/command/tag.py | 156 --------------------------------- dvc/dvcfile.py | 7 +- dvc/output/__init__.py | 17 +--- dvc/output/base.py | 15 +--- dvc/repo/__init__.py | 15 ++-- dvc/repo/lock.py | 4 +- dvc/repo/remove.py | 2 +- dvc/repo/reproduce.py | 4 +- dvc/repo/tag/__init__.py | 18 ---- dvc/repo/tag/add.py | 24 ----- dvc/repo/tag/list.py | 15 ---- dvc/repo/tag/remove.py | 23 ----- dvc/stage/__init__.py | 3 - dvc/stage/loader.py | 30 +++++-- dvc/utils/__init__.py | 15 ++-- tests/func/test_tag.py | 120 ------------------------- tests/unit/utils/test_utils.py | 16 ++-- 19 files changed, 60 insertions(+), 434 deletions(-) delete mode 100644 dvc/command/tag.py delete mode 100644 dvc/repo/tag/__init__.py delete mode 100644 dvc/repo/tag/add.py delete mode 100644 dvc/repo/tag/list.py delete mode 100644 dvc/repo/tag/remove.py delete mode 100644 tests/func/test_tag.py diff --git a/dvc/cli.py b/dvc/cli.py index 29ccc4613d..0e4a22a25c 100644 --- a/dvc/cli.py +++ b/dvc/cli.py @@ -31,7 +31,6 @@ repro, root, run, - tag, unprotect, update, version, @@ -71,7 +70,6 @@ pipeline, daemon, commit, - tag, diff, version, update, diff --git a/dvc/command/pipeline.py b/dvc/command/pipeline.py index 1eb7b281c5..d788031660 100644 --- a/dvc/command/pipeline.py +++ b/dvc/command/pipeline.py @@ -14,8 +14,8 @@ def _show(self, target, commands, outs, locked): from dvc import dvcfile from dvc.utils import parse_target - path, name, tag = parse_target(target) - stage = dvcfile.Dvcfile(self.repo, path, tag=tag).stages[name] + path, name = parse_target(target) + stage = dvcfile.Dvcfile(self.repo, path).stages[name] G = self.repo.graph stages = networkx.dfs_postorder_nodes(G, stage) if locked: @@ -38,8 +38,8 @@ def _build_graph(self, target, commands=False, outs=False): from dvc.repo.graph import get_pipeline from dvc.utils import parse_target - path, name, tag = parse_target(target) - target_stage = dvcfile.Dvcfile(self.repo, path, tag=tag).stages[name] + path, name = parse_target(target) + target_stage = dvcfile.Dvcfile(self.repo, path).stages[name] G = get_pipeline(self.repo.pipelines, target_stage) nodes = set() diff --git a/dvc/command/tag.py b/dvc/command/tag.py deleted file mode 100644 index 859ea91249..0000000000 --- a/dvc/command/tag.py +++ /dev/null @@ -1,156 +0,0 @@ -import logging - -from dvc.command.base import append_doc_link -from dvc.command.base import CmdBase -from dvc.command.base import fix_subparsers -from dvc.exceptions import DvcException -from dvc.utils import to_yaml_string - - -logger = logging.getLogger(__name__) - - -class CmdTagAdd(CmdBase): - def run(self): - for target in self.args.targets: - try: - self.repo.tag.add( - self.args.tag, - target=target, - with_deps=self.args.with_deps, - recursive=self.args.recursive, - ) - except DvcException: - logger.exception("failed to add tag") - return 1 - return 0 - - -class CmdTagRemove(CmdBase): - def run(self): - for target in self.args.targets: - try: - self.repo.tag.remove( - self.args.tag, - target=target, - with_deps=self.args.with_deps, - recursive=self.args.recursive, - ) - except DvcException: - logger.exception("failed to remove tag") - return 1 - return 0 - - -class CmdTagList(CmdBase): - def run(self): - for target in self.args.targets: - try: - tags = self.repo.tag.list( - target, - with_deps=self.args.with_deps, - recursive=self.args.recursive, - ) - if tags: - logger.info(to_yaml_string(tags)) - except DvcException: - logger.exception("failed list tags") - return 1 - return 0 - - -def add_parser(subparsers, parent_parser): - TAG_HELP = "A set of commands to manage DVC tags." - tag_parser = subparsers.add_parser( - "tag", - parents=[parent_parser], - description=append_doc_link(TAG_HELP, "tag"), - add_help=False, - ) - - tag_subparsers = tag_parser.add_subparsers( - dest="cmd", - help="Use DVC tag CMD --help to display command-specific help.", - ) - - fix_subparsers(tag_subparsers) - - TAG_ADD_HELP = "Add DVC tag." - tag_add_parser = tag_subparsers.add_parser( - "add", - parents=[parent_parser], - description=append_doc_link(TAG_ADD_HELP, "tag-add"), - help=TAG_ADD_HELP, - ) - tag_add_parser.add_argument("tag", help="Dvc tag.") - tag_add_parser.add_argument( - "targets", nargs="*", default=[None], help="DVC-files." - ) - tag_add_parser.add_argument( - "-d", - "--with-deps", - action="store_true", - default=False, - help="Add tag for all dependencies of the specified DVC-file.", - ) - tag_add_parser.add_argument( - "-R", - "--recursive", - action="store_true", - default=False, - help="Add tag for subdirectories of the specified directory.", - ) - tag_add_parser.set_defaults(func=CmdTagAdd) - - TAG_REMOVE_HELP = "Remove DVC tag." - tag_remove_parser = tag_subparsers.add_parser( - "remove", - parents=[parent_parser], - description=append_doc_link(TAG_REMOVE_HELP, "tag-remove"), - help=TAG_REMOVE_HELP, - ) - tag_remove_parser.add_argument("tag", help="Dvc tag.") - tag_remove_parser.add_argument( - "targets", nargs="*", default=[None], help="DVC-files." - ) - tag_remove_parser.add_argument( - "-d", - "--with-deps", - action="store_true", - default=False, - help="Remove tag for all dependencies of the specified DVC-file.", - ) - tag_remove_parser.add_argument( - "-R", - "--recursive", - action="store_true", - default=False, - help="Remove tag for subdirectories of the specified directory.", - ) - tag_remove_parser.set_defaults(func=CmdTagRemove) - - TAG_LIST_HELP = "List DVC tags." - tag_list_parser = tag_subparsers.add_parser( - "list", - parents=[parent_parser], - description=append_doc_link(TAG_LIST_HELP, "tag-list"), - help=TAG_LIST_HELP, - ) - tag_list_parser.add_argument( - "targets", nargs="*", default=[None], help="DVC-files." - ) - tag_list_parser.add_argument( - "-d", - "--with-deps", - action="store_true", - default=False, - help="List tags for all dependencies of the specified DVC-file.", - ) - tag_list_parser.add_argument( - "-R", - "--recursive", - action="store_true", - default=False, - help="List tags for subdirectories of the specified directory.", - ) - tag_list_parser.set_defaults(func=CmdTagList) diff --git a/dvc/dvcfile.py b/dvc/dvcfile.py index 9eb35d1942..3e8f6b0ae4 100644 --- a/dvc/dvcfile.py +++ b/dvc/dvcfile.py @@ -128,19 +128,18 @@ def dump(self, stage, **kwargs): class SingleStageFile(FileMixin): from dvc.schema import COMPILED_SINGLE_STAGE_SCHEMA as SCHEMA - def __init__(self, repo, path, tag=None): + def __init__(self, repo, path): super().__init__(repo, path) - self.tag = tag @property def stage(self): data, raw = self._load() - return SingleStageLoader.load_stage(self, data, raw, tag=self.tag) + return SingleStageLoader.load_stage(self, data, raw) @property def stages(self): data, raw = self._load() - return SingleStageLoader(self, data, raw, tag=self.tag) + return SingleStageLoader(self, data, raw) def dump(self, stage, **kwargs): """Dumps given stage appropriately in the dvcfile.""" diff --git a/dvc/output/__init__.py b/dvc/output/__init__.py index 00533ca791..60c7145dd1 100644 --- a/dvc/output/__init__.py +++ b/dvc/output/__init__.py @@ -49,17 +49,14 @@ HDFSRemote.PARAM_CHECKSUM: CHECKSUM_SCHEMA, } -TAGS_SCHEMA = {str: CHECKSUMS_SCHEMA} - SCHEMA = CHECKSUMS_SCHEMA.copy() SCHEMA[Required(BaseOutput.PARAM_PATH)] = str SCHEMA[BaseOutput.PARAM_CACHE] = bool SCHEMA[BaseOutput.PARAM_METRIC] = BaseOutput.METRIC_SCHEMA -SCHEMA[BaseOutput.PARAM_TAGS] = TAGS_SCHEMA SCHEMA[BaseOutput.PARAM_PERSIST] = bool -def _get(stage, p, info, cache, metric, persist=False, tags=None): +def _get(stage, p, info, cache, metric, persist=False): parsed = urlparse(p) if parsed.scheme == "remote": @@ -72,7 +69,6 @@ def _get(stage, p, info, cache, metric, persist=False, tags=None): remote=remote, metric=metric, persist=persist, - tags=tags, ) for o in OUTS: @@ -85,7 +81,6 @@ def _get(stage, p, info, cache, metric, persist=False, tags=None): remote=None, metric=metric, persist=persist, - tags=tags, ) return LocalOutput( stage, @@ -95,7 +90,6 @@ def _get(stage, p, info, cache, metric, persist=False, tags=None): remote=None, metric=metric, persist=persist, - tags=tags, ) @@ -106,16 +100,9 @@ def loadd_from(stage, d_list): cache = d.pop(BaseOutput.PARAM_CACHE, True) metric = d.pop(BaseOutput.PARAM_METRIC, False) persist = d.pop(BaseOutput.PARAM_PERSIST, False) - tags = d.pop(BaseOutput.PARAM_TAGS, None) ret.append( _get( - stage, - p, - info=d, - cache=cache, - metric=metric, - persist=persist, - tags=tags, + stage, p, info=d, cache=cache, metric=metric, persist=persist, ) ) return ret diff --git a/dvc/output/base.py b/dvc/output/base.py index b70215449b..8b57517f8c 100644 --- a/dvc/output/base.py +++ b/dvc/output/base.py @@ -59,8 +59,6 @@ class BaseOutput(object): }, ) - PARAM_TAGS = "tags" - DoesNotExistError = OutputDoesNotExistError IsNotFileOrDirError = OutputIsNotFileOrDirError IsStageFileError = OutputIsStageFileError @@ -76,7 +74,6 @@ def __init__( cache=True, metric=False, persist=False, - tags=None, ): self._validate_output_path(path) # This output (and dependency) objects have too many paths/urls @@ -97,7 +94,6 @@ def __init__( self.use_cache = False if self.IS_DEPENDENCY else cache self.metric = False if self.IS_DEPENDENCY else metric self.persist = persist - self.tags = None if self.IS_DEPENDENCY else (tags or {}) self.path_info = self._parse_path(remote, path) if self.use_cache and self.cache is None: @@ -274,9 +270,6 @@ def dumpd(self): ret[self.PARAM_METRIC] = self.metric ret[self.PARAM_PERSIST] = self.persist - if self.tags: - ret[self.PARAM_TAGS] = self.tags - return ret def verify_metric(self): @@ -291,7 +284,6 @@ def checkout( self, force=False, progress_callback=None, - tag=None, relink=False, filter_info=None, ): @@ -302,14 +294,9 @@ def checkout( ) return None - if tag: - info = self.tags[tag] - else: - info = self.info - return self.cache.checkout( self.path_info, - info, + self.info, force=force, progress_callback=progress_callback, relink=relink, diff --git a/dvc/repo/__init__.py b/dvc/repo/__init__.py index 9040e0dffc..e146b64ad0 100644 --- a/dvc/repo/__init__.py +++ b/dvc/repo/__init__.py @@ -1,3 +1,4 @@ +import logging import os from contextlib import contextmanager from functools import wraps @@ -70,7 +71,6 @@ def __init__(self, root_dir=None): from dvc.repo.metrics import Metrics from dvc.repo.params import Params from dvc.scm.tree import WorkingTree - from dvc.repo.tag import Tag from dvc.utils.fs import makedirs root_dir = self.find_root(root_dir) @@ -106,7 +106,6 @@ def __init__(self, root_dir=None): self.metrics = Metrics(self) self.params = Params(self) - self.tag = Tag(self) self._ignore() @@ -211,8 +210,8 @@ def collect(self, target, with_deps=False, recursive=False, graph=None): os.path.abspath(target), graph or self.graph ) - file, name, tag = parse_target(target) - dvcfile = Dvcfile(self, file, tag=tag) + path, name = parse_target(target) + dvcfile = Dvcfile(self, path) stages = list(dvcfile.stages.filter(name).values()) if not with_deps: return stages @@ -229,10 +228,10 @@ def collect_granular(self, target, *args, **kwargs): if not target: return [(stage, None) for stage in self.stages] - file, name, tag = parse_target(target) + file, name = parse_target(target) if is_valid_filename(file) and not kwargs.get("with_deps"): # Optimization: do not collect the graph for a specific .dvc target - stages = Dvcfile(self, file, tag=tag).stages.filter(name) + stages = Dvcfile(self, file).stages.filter(name) return [(stage, None) for stage in stages.values()] try: @@ -433,7 +432,9 @@ def _collect_stages(self): for root, dirs, files in self.tree.walk(self.root_dir): for file_name in filter(is_valid_filename, files): path = os.path.join(root, file_name) - stages.extend(list(Dvcfile(self, path).stages.values())) + stage_loader = Dvcfile(self, path).stages + with stage_loader.log_level(at=logging.DEBUG): + stages.extend(stage_loader.values()) outs.update( out.fspath for stage in stages diff --git a/dvc/repo/lock.py b/dvc/repo/lock.py index 7845d34931..b3d006950a 100644 --- a/dvc/repo/lock.py +++ b/dvc/repo/lock.py @@ -6,8 +6,8 @@ def lock(self, target, unlock=False): from .. import dvcfile from dvc.utils import parse_target - path, name, tag = parse_target(target) - dvcfile = dvcfile.Dvcfile(self, path, tag=tag) + path, name = parse_target(target) + dvcfile = dvcfile.Dvcfile(self, path) stage = dvcfile.stages[name] stage.locked = False if unlock else True dvcfile.dump(stage, update_pipeline=True) diff --git a/dvc/repo/remove.py b/dvc/repo/remove.py index f807270441..4603dd9e9f 100644 --- a/dvc/repo/remove.py +++ b/dvc/repo/remove.py @@ -10,7 +10,7 @@ def remove(self, target, outs_only=False): from ..dvcfile import Dvcfile - path, name, _ = parse_target(target) + path, name = parse_target(target) dvcfile = Dvcfile(self, path) stages = list(dvcfile.stages.filter(name).values()) for stage in stages: diff --git a/dvc/repo/reproduce.py b/dvc/repo/reproduce.py index 288bdd39aa..8bf4f84d12 100644 --- a/dvc/repo/reproduce.py +++ b/dvc/repo/reproduce.py @@ -76,12 +76,12 @@ def reproduce( active_graph = _get_active_graph(self.graph) active_pipelines = get_pipelines(active_graph) - path, name, tag = parse_target(target) + path, name = parse_target(target) if pipeline or all_pipelines: if all_pipelines: pipelines = active_pipelines else: - dvcfile = Dvcfile(self, path, tag=tag) + dvcfile = Dvcfile(self, path) stage = dvcfile.stages[name] pipelines = [get_pipeline(active_pipelines, stage)] diff --git a/dvc/repo/tag/__init__.py b/dvc/repo/tag/__init__.py deleted file mode 100644 index 7cd64dea8c..0000000000 --- a/dvc/repo/tag/__init__.py +++ /dev/null @@ -1,18 +0,0 @@ -class Tag(object): - def __init__(self, repo): - self.repo = repo - - def add(self, *args, **kwargs): - from dvc.repo.tag.add import add - - return add(self.repo, *args, **kwargs) - - def list(self, *args, **kwargs): - from dvc.repo.tag.list import list - - return list(self.repo, *args, **kwargs) - - def remove(self, *args, **kwargs): - from dvc.repo.tag.remove import remove - - return remove(self.repo, *args, **kwargs) diff --git a/dvc/repo/tag/add.py b/dvc/repo/tag/add.py deleted file mode 100644 index 38875d6e1c..0000000000 --- a/dvc/repo/tag/add.py +++ /dev/null @@ -1,24 +0,0 @@ -import logging -from copy import copy - -from dvc.repo import locked -from dvc.dvcfile import Dvcfile - - -logger = logging.getLogger(__name__) - - -@locked -def add(self, tag, target=None, with_deps=False, recursive=False): - stages = self.collect(target, with_deps=with_deps, recursive=recursive) - for stage in stages: - changed = False - for out in stage.outs: - if not out.info: - logger.warning("missing checksum info for '{}'".format(out)) - continue - out.tags[tag] = copy(out.info) - changed = True - if changed: - dvcfile = Dvcfile(self, stage.path) - dvcfile.dump(stage) diff --git a/dvc/repo/tag/list.py b/dvc/repo/tag/list.py deleted file mode 100644 index 487d5afdbd..0000000000 --- a/dvc/repo/tag/list.py +++ /dev/null @@ -1,15 +0,0 @@ -from dvc.repo import locked - - -@locked -def list(self, target=None, with_deps=False, recursive=False): - ret = {} - stages = self.collect(target, with_deps=with_deps, recursive=recursive) - for stage in stages: - outs_tags = {} - for out in stage.outs: - if out.tags: - outs_tags[str(out)] = out.tags.copy() - if outs_tags: - ret.update({stage.relpath: outs_tags}) - return ret diff --git a/dvc/repo/tag/remove.py b/dvc/repo/tag/remove.py deleted file mode 100644 index f8fb5638ce..0000000000 --- a/dvc/repo/tag/remove.py +++ /dev/null @@ -1,23 +0,0 @@ -import logging - -from dvc.repo import locked -from dvc.dvcfile import Dvcfile - - -logger = logging.getLogger(__name__) - - -@locked -def remove(self, tag, target=None, with_deps=False, recursive=False): - stages = self.collect(target, with_deps=with_deps, recursive=recursive) - for stage in stages: - changed = False - for out in stage.outs: - if tag not in out.tags.keys(): - logger.warning("tag '{}' not found for '{}'".format(tag, out)) - continue - del out.tags[tag] - changed = True - if changed: - dvcfile = Dvcfile(self, stage.path) - dvcfile.dump(stage) diff --git a/dvc/stage/__init__.py b/dvc/stage/__init__.py index d79753dc11..e89c42231b 100644 --- a/dvc/stage/__init__.py +++ b/dvc/stage/__init__.py @@ -112,7 +112,6 @@ def __init__( self.deps = deps self.md5 = md5 self.locked = locked - self.tag = tag self.always_changed = always_changed self._stage_text = stage_text self._dvcfile = dvcfile @@ -478,7 +477,6 @@ def _compute_md5(self): exclude=[ self.PARAM_LOCKED, BaseOutput.PARAM_METRIC, - BaseOutput.PARAM_TAGS, BaseOutput.PARAM_PERSIST, ], ) @@ -694,7 +692,6 @@ def checkout( try: result = out.checkout( force=force, - tag=self.tag, progress_callback=progress_callback, relink=relink, filter_info=filter_info, diff --git a/dvc/stage/loader.py b/dvc/stage/loader.py index aefe42789b..02912842d5 100644 --- a/dvc/stage/loader.py +++ b/dvc/stage/loader.py @@ -1,5 +1,6 @@ import logging import os +from contextlib import contextmanager from copy import deepcopy from collections import defaultdict, Mapping @@ -29,6 +30,14 @@ def __init__(self, dvcfile, stages_data, lockfile_data=None): self.dvcfile = dvcfile self.stages_data = stages_data or {} self.lockfile_data = lockfile_data or {} + self._log_level = logging.WARNING + + @contextmanager + def log_level(self, at): + """Change log_level temporarily for StageLoader.""" + self._log_level, level = at, self._log_level + yield + self._log_level = level def filter(self, item=None): if not item: @@ -147,8 +156,11 @@ def __getitem__(self, name): raise StageNotFound(self.dvcfile, name) if not self.lockfile_data.get(name): - logger.warning( - "No lock entry found for '%s:%s'", self.dvcfile.relpath, name + logger.log( + self._log_level, + "No lock entry found for '%s:%s'", + self.dvcfile.relpath, + name, ) return self.load_stage( @@ -169,15 +181,19 @@ def __contains__(self, name): class SingleStageLoader(Mapping): - def __init__(self, dvcfile, stage_data, stage_text=None, tag=None): + def __init__(self, dvcfile, stage_data, stage_text=None): self.dvcfile = dvcfile self.stage_data = stage_data or {} self.stage_text = stage_text - self.tag = tag def filter(self, item=None): return self + @contextmanager + def log_level(self, *args, **kwargs): + """No-op context manager.""" + yield + def __getitem__(self, item): if item: logger.warning( @@ -188,16 +204,16 @@ def __getitem__(self, item): # during `load`, we remove attributes from stage data, so as to # not duplicate, therefore, for MappingView, we need to deepcopy. return self.load_stage( - self.dvcfile, deepcopy(self.stage_data), self.stage_text, self.tag + self.dvcfile, deepcopy(self.stage_data), self.stage_text ) @classmethod - def load_stage(cls, dvcfile, d, stage_text, tag=None): + def load_stage(cls, dvcfile, d, stage_text): from dvc.stage import Stage, loads_from path, wdir = resolve_paths(dvcfile.path, d.get(Stage.PARAM_WDIR)) stage = loads_from(Stage, dvcfile.repo, path, wdir, d) - stage._stage_text, stage.tag = stage_text, tag + stage._stage_text = stage_text stage.deps = dependency.loadd_from( stage, d.get(Stage.PARAM_DEPS) or [] ) diff --git a/dvc/utils/__init__.py b/dvc/utils/__init__.py index 3365646129..56260b4a4a 100644 --- a/dvc/utils/__init__.py +++ b/dvc/utils/__init__.py @@ -23,9 +23,7 @@ LOCAL_CHUNK_SIZE = 2 ** 20 # 1 MB LARGE_FILE_SIZE = 2 ** 30 # 1 GB LARGE_DIR_SIZE = 100 -TARGET_REGEX = re.compile( - r"(?P.*?)(:(?P[^\\/@:]*))??(@(?P[^\\/@:]*))??$" -) +TARGET_REGEX = re.compile(r"(?P.*?)(:(?P[^\\/@:]*))??$") def dos2unix(data): @@ -384,24 +382,23 @@ def parse_target(target, default=None): from dvc.exceptions import DvcException if not target: - return None, None, None + return None, None match = TARGET_REGEX.match(target) if not match: - return target, None, None + return target, None - path, name, tag = ( + path, name = ( match.group("path"), match.group("name"), - match.group("tag"), ) if not path: path = default or PIPELINE_FILE - logger.warning("Assuming file to be '%s'", path) + logger.debug("Assuming file to be '%s'", path) if os.path.basename(path) == PIPELINE_LOCK: raise DvcException( "Did you mean: `{}`?".format(target.replace(".lock", ".yaml", 1)) ) - return path, name, tag + return path, name diff --git a/tests/func/test_tag.py b/tests/func/test_tag.py deleted file mode 100644 index 13b67bd031..0000000000 --- a/tests/func/test_tag.py +++ /dev/null @@ -1,120 +0,0 @@ -import filecmp -import logging -import os -import shutil - -from dvc.main import main -from dvc.utils import from_yaml_string -from tests.basic_env import TestDvc - - -class TestTag(TestDvc): - def test(self): - fname = "file" - shutil.copyfile(self.FOO, fname) - - stages = self.dvc.add(fname) - self.assertEqual(len(stages), 1) - stage = stages[0] - self.assertTrue(stage is not None) - - ret = main(["tag", "add", "v1", stage.path]) - self.assertEqual(ret, 0) - - os.unlink(fname) - shutil.copyfile(self.BAR, fname) - - ret = main(["repro", stage.path]) - self.assertEqual(ret, 0) - - ret = main(["tag", "add", "v2", stage.path]) - self.assertEqual(ret, 0) - - ret = main(["tag", "list", stage.path]) - self.assertEqual(ret, 0) - - ret = main(["checkout", stage.path + "@v1"]) - self.assertEqual(ret, 0) - - self.assertTrue(filecmp.cmp(fname, self.FOO, shallow=False)) - - ret = main(["checkout", stage.path + "@v2"]) - self.assertEqual(ret, 0) - - self.assertTrue(filecmp.cmp(fname, self.BAR, shallow=False)) - - -class TestTagAll(TestDvc): - def test(self): - with self._caplog.at_level(logging.INFO, logger="dvc"): - ret = main(["add", self.FOO, self.BAR]) - self.assertEqual(ret, 0) - - self._caplog.clear() - - ret = main(["tag", "list"]) - self.assertEqual(ret, 0) - - self.assertEqual("", self._caplog.text) - - ret = main(["tag", "add", "v1"]) - self.assertEqual(ret, 0) - - self._caplog.clear() - - ret = main(["tag", "list"]) - self.assertEqual(ret, 0) - - expected = { - "foo.dvc": { - "foo": {"v1": {"md5": "acbd18db4cc2f85cedef654fccc4a4d8"}} - }, - "bar.dvc": { - "bar": {"v1": {"md5": "8978c98bb5a48c2fb5f2c4c905768afa"}} - }, - } - - stdout = "\n".join( - record.message for record in self._caplog.records - ) - received = from_yaml_string(stdout) - - assert expected == received - - ret = main(["tag", "remove", "v1"]) - self.assertEqual(ret, 0) - - self._caplog.clear() - - ret = main(["tag", "list"]) - self.assertEqual(ret, 0) - - self.assertEqual("", self._caplog.text) - - -class TestTagAddNoChecksumInfo(TestDvc): - def test(self): - ret = main(["run", "-o", self.FOO, "--no-exec"]) - self.assertEqual(ret, 0) - - self._caplog.clear() - - with self._caplog.at_level(logging.WARNING, logger="dvc"): - ret = main(["tag", "add", "v1", "foo.dvc"]) - self.assertEqual(ret, 0) - - assert "missing checksum info for 'foo'" in self._caplog.text - - -class TestTagRemoveNoTag(TestDvc): - def test(self): - ret = main(["add", self.FOO]) - self.assertEqual(ret, 0) - - self._caplog.clear() - - with self._caplog.at_level(logging.WARNING, logger="dvc"): - ret = main(["tag", "remove", "v1", "foo.dvc"]) - self.assertEqual(ret, 0) - - assert "tag 'v1' not found for 'foo'" in self._caplog.text diff --git a/tests/unit/utils/test_utils.py b/tests/unit/utils/test_utils.py index da7194d5f9..5f9b7f1e2a 100644 --- a/tests/unit/utils/test_utils.py +++ b/tests/unit/utils/test_utils.py @@ -137,14 +137,14 @@ def test_resolve_output(inp, out, is_dir, expected, mocker): @pytest.mark.parametrize( "inp,out, default", [ - ["pipelines.yaml", ("pipelines.yaml", None, None), None], - ["pipelines.yaml:name", ("pipelines.yaml", "name", None), None], - [":name", ("pipelines.yaml", "name", None), None], - ["stage.dvc", ("stage.dvc", None, None), None], - ["pipelines.yaml:name@v1", ("pipelines.yaml", "name", "v1"), None], - ["../models/stage.dvc", ("../models/stage.dvc", None, None), "def"], - [":name", ("default", "name", None), "default"], - [":name@v2", ("default", "name", "v2"), "default"], + ["pipelines.yaml", ("pipelines.yaml", None), None], + ["pipelines.yaml:name", ("pipelines.yaml", "name"), None], + [":name", ("pipelines.yaml", "name"), None], + ["stage.dvc", ("stage.dvc", None), None], + ["pipelines.yaml:name", ("pipelines.yaml", "name"), None], + ["../models/stage.dvc", ("../models/stage.dvc", None), "def"], + [":name", ("default", "name"), "default"], + [":name", ("default", "name"), "default"], ], ) def test_parse_target(inp, out, default): From 539bd382e4977493a13281e3cd6df47f6627743e Mon Sep 17 00:00:00 2001 From: Saugat Pachhai Date: Wed, 29 Apr 2020 18:16:02 +0545 Subject: [PATCH 2/4] tests: adjust for tags and allow @ in stage name --- dvc/repo/run.py | 2 +- dvc/stage/exceptions.py | 2 +- dvc/utils/__init__.py | 2 +- tests/unit/utils/test_utils.py | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/dvc/repo/run.py b/dvc/repo/run.py index 63e76b5c0f..55b75b8cac 100644 --- a/dvc/repo/run.py +++ b/dvc/repo/run.py @@ -10,7 +10,7 @@ def is_valid_name(name: str): - return not {"\\", "/", "@", ":"} & set(name) + return not {"\\", "/", ":"} & set(name) def _get_file_path(kwargs): diff --git a/dvc/stage/exceptions.py b/dvc/stage/exceptions.py index 3049826c4a..c41d7968fb 100644 --- a/dvc/stage/exceptions.py +++ b/dvc/stage/exceptions.py @@ -124,5 +124,5 @@ class InvalidStageName(DvcException): def __init__(self,): super().__init__( "Stage name cannot contain invalid characters: " - "'\\', '/', '@' and ':'." + "'\\', '/', and ':'." ) diff --git a/dvc/utils/__init__.py b/dvc/utils/__init__.py index 56260b4a4a..2c03f7db25 100644 --- a/dvc/utils/__init__.py +++ b/dvc/utils/__init__.py @@ -23,7 +23,7 @@ LOCAL_CHUNK_SIZE = 2 ** 20 # 1 MB LARGE_FILE_SIZE = 2 ** 30 # 1 GB LARGE_DIR_SIZE = 100 -TARGET_REGEX = re.compile(r"(?P.*?)(:(?P[^\\/@:]*))??$") +TARGET_REGEX = re.compile(r"(?P.*?)(:(?P[^\\/:]*))??$") def dos2unix(data): diff --git a/tests/unit/utils/test_utils.py b/tests/unit/utils/test_utils.py index 5f9b7f1e2a..8e18cb5422 100644 --- a/tests/unit/utils/test_utils.py +++ b/tests/unit/utils/test_utils.py @@ -153,5 +153,5 @@ def test_parse_target(inp, out, default): def test_hint_on_lockfile(): with pytest.raises(Exception) as exc: - assert parse_target("pipelines.lock:name@v223") - assert "pipelines.yaml:name@v223" in str(exc.value) + assert parse_target("pipelines.lock:name") + assert "pipelines.yaml:name" in str(exc.value) From f7c968f560909436478727c17f4ad4e88274d772 Mon Sep 17 00:00:00 2001 From: Saugat Pachhai Date: Wed, 29 Apr 2020 18:32:59 +0545 Subject: [PATCH 3/4] disallow @ on stage name --- dvc/repo/run.py | 2 +- dvc/stage/exceptions.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dvc/repo/run.py b/dvc/repo/run.py index 55b75b8cac..63e76b5c0f 100644 --- a/dvc/repo/run.py +++ b/dvc/repo/run.py @@ -10,7 +10,7 @@ def is_valid_name(name: str): - return not {"\\", "/", ":"} & set(name) + return not {"\\", "/", "@", ":"} & set(name) def _get_file_path(kwargs): diff --git a/dvc/stage/exceptions.py b/dvc/stage/exceptions.py index c41d7968fb..3049826c4a 100644 --- a/dvc/stage/exceptions.py +++ b/dvc/stage/exceptions.py @@ -124,5 +124,5 @@ class InvalidStageName(DvcException): def __init__(self,): super().__init__( "Stage name cannot contain invalid characters: " - "'\\', '/', and ':'." + "'\\', '/', '@' and ':'." ) From c5da7e5a4cf01af5afe1dfbd1c6f92018b65a1b5 Mon Sep 17 00:00:00 2001 From: Saugat Pachhai Date: Wed, 29 Apr 2020 19:10:43 +0545 Subject: [PATCH 4/4] fix ds issue, leftover tag in Stage --- dvc/stage/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/dvc/stage/__init__.py b/dvc/stage/__init__.py index e89c42231b..fcdbeed671 100644 --- a/dvc/stage/__init__.py +++ b/dvc/stage/__init__.py @@ -94,7 +94,6 @@ def __init__( outs=None, md5=None, locked=False, - tag=None, always_changed=False, stage_text=None, dvcfile=None,