From 36323010fc4be9a63e51627c82e03dffcac74720 Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Tue, 27 Feb 2018 01:36:20 +0200 Subject: [PATCH 1/7] tests: add test for removing directory Signed-off-by: Ruslan Kuprieiev --- tests/test_remove.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/test_remove.py b/tests/test_remove.py index 59f2d0c50..2de7fa4cb 100644 --- a/tests/test_remove.py +++ b/tests/test_remove.py @@ -31,6 +31,17 @@ class TestRemoveFileOutsideOfRepo(TestDvc): self.dvc.remove(os.path.join(os.path.dirname(self.dvc.root_dir), self.FOO)) +class TestRemoveDirectory(TestDvc): + def test(self): + stage_add = self.dvc.add(self.DATA_DIR) + stages_removed = self.dvc.remove(self.DATA_DIR) + self.assertEqual(len(stages_removed), 1) + stage_removed = stages_removed[0] + self.assertEqual(stage_add.path, stage_removed.path) + self.assertFalse(os.path.exists(self.DATA_DIR)) + self.assertFalse(os.path.exists(stage_removed.path)) + + class TestCmdRemove(TestDvc): def test(self): self.dvc.add(self.FOO) From a05eddd746317f3274763a8eb3f30c2fecca0754 Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Tue, 27 Feb 2018 02:25:57 +0200 Subject: [PATCH 2/7] tests: add test for cli data cloud Signed-off-by: Ruslan Kuprieiev --- tests/test_data_cloud.py | 51 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/tests/test_data_cloud.py b/tests/test_data_cloud.py index 9bbd908df..e86efa961 100644 --- a/tests/test_data_cloud.py +++ b/tests/test_data_cloud.py @@ -3,6 +3,7 @@ import stat import uuid import shutil +from dvc.main import main from dvc.config import ConfigError from dvc.cloud.data_cloud import DataCloud, DataCloudAWS, DataCloudLOCAL, DataCloudGCP from dvc.cloud.base import STATUS_UNKNOWN, STATUS_OK, STATUS_MODIFIED, STATUS_NEW, STATUS_DELETED @@ -178,3 +179,53 @@ class TestDataCloudLOCAL(TestDataCloudBase): if self._should_test(): self._test_cloud() self.assertTrue(os.path.isdir(self.dname)) + + +class TestDataCloudLocalCli(TestDvc): + def main(self, args): + ret = main(args) + self.assertEqual(ret, 0) + + def test(self): + #FIXME enable this test for windows + if os.name == 'nt': + return + + dname = 'cloud' + os.mkdir(dname) + + self.main(['config', 'global.cloud', 'LOCAL']) + self.main(['config', 'LOCAL.StoragePath', dname]) + + stage = self.dvc.add(self.FOO) + cache = stage.outs[0].cache + + stage_dir = self.dvc.add(self.DATA_DIR) + cache_dir = stage_dir.outs[0].cache + + #FIXME check status output + self.main(['cache', 'status']) + + self.main(['cache', 'push']) + self.assertTrue(os.path.exists(cache)) + self.assertTrue(os.path.isfile(cache)) + self.assertTrue(os.path.exists(cache_dir)) + self.assertTrue(os.path.isdir(cache_dir)) + + self.main(['cache', 'status']) + + os.chmod(cache, 0o777) + os.unlink(cache) + self.dvc._remove_cache(cache_dir) + + self.main(['cache', 'status']) + + self.main(['cache', 'pull']) + self.assertTrue(os.path.exists(cache)) + self.assertTrue(os.path.isfile(cache)) + with open(cache, 'r') as fd: + self.assertEqual(fd.read(), self.FOO_CONTENTS) + self.assertTrue(os.path.exists(cache_dir)) + self.assertTrue(os.path.isdir(cache_dir)) + + self.main(['cache', 'status']) From bdb55cce7c422bdaf624f23be2bc2729afaf09b7 Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Tue, 27 Feb 2018 02:34:17 +0200 Subject: [PATCH 3/7] utils: remove unused parse_target_metrics Signed-off-by: Ruslan Kuprieiev --- dvc/utils.py | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/dvc/utils.py b/dvc/utils.py index 55e4c037a..19e022e23 100644 --- a/dvc/utils.py +++ b/dvc/utils.py @@ -88,27 +88,3 @@ def map_progress(func, targets, n_threads): progress.finish() return list(zip(targets, ret)) - - -def parse_target_metric_file(file_name): - with open(file_name, 'r') as fd: - try: - lines = fd.readlines(2) - except Exception: - return None - return parse_target_metric(lines) - - -FLOATS_FROM_STRING = re.compile(r'[-+]?(?:(?:\d*\.\d+)|(?:\d+\.?))(?:[Ee][+-]?\d+)?') - - -def parse_target_metric(lines): - if len(lines) != 1: - return None - - # Extract float from string. I.e. from 'AUC: 0.596182' - nums = FLOATS_FROM_STRING.findall(lines[0]) - if len(nums) < 1: - return None - - return float(nums[0]) From 635290895980281a4a2916290cae0c0f0743424c Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Tue, 27 Feb 2018 02:38:21 +0200 Subject: [PATCH 4/7] cloud: remove unused remove() Signed-off-by: Ruslan Kuprieiev --- dvc/cloud/base.py | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/dvc/cloud/base.py b/dvc/cloud/base.py index 262604783..a846ee792 100644 --- a/dvc/cloud/base.py +++ b/dvc/cloud/base.py @@ -169,26 +169,6 @@ class DataCloudBase(object): return res - def remove(self, path): - """ - Generic method for removing data item from the cloud. - """ - key = self._get_key(path) - if key: - key.delete() - return [k.name] - - keys = self._get_keys(path) - if not keys: - Logger.error("File '{}' does not exist in the cloud".format(path)) - return [] - - res = [] - for k in keys: - k.delete() - res.append(k.name) - return res - def _status(self, key, path): remote_exists = key != None local_exists = os.path.exists(path) From d87e8e47af6b981bb60ff1171100f13f89c4b28e Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Tue, 27 Feb 2018 02:51:57 +0200 Subject: [PATCH 5/7] test: add test for non-existing sections/fields in config Signed-off-by: Ruslan Kuprieiev --- tests/test_config.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/test_config.py b/tests/test_config.py index d9818f936..96c9ae60d 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -170,6 +170,9 @@ class TestConfigCLI(TestDvc): self.assertEqual(ret, 0) self.assertTrue(self._contains(section, field, value)) + ret = main(['config', section_field]) + self.assertEqual(ret, 0) + ret = main(['config', section_field, newvalue]) self.assertEqual(ret, 0) self.assertTrue(self._contains(section, field, newvalue)) @@ -178,3 +181,18 @@ class TestConfigCLI(TestDvc): ret = main(['config', section_field, '--unset']) self.assertEqual(ret, 0) self.assertFalse(self._contains(section, field, value)) + + def test_non_existing(self): + #FIXME check stdout/err + + ret = main(['config', 'non_existing_section.field']) + self.assertEqual(ret, 1) + + ret = main(['config', 'global.non_existing_field']) + self.assertEqual(ret, 1) + + ret = main(['config', 'non_existing_section.field', '-u']) + self.assertEqual(ret, 1) + + ret = main(['config', 'global.non_existing_field', '-u']) + self.assertEqual(ret, 1) From 713f070ef89fe0aa756617608d9e82ba3ff6a2e2 Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Tue, 27 Feb 2018 02:59:03 +0200 Subject: [PATCH 6/7] dvc: remove unused pipeline code Signed-off-by: Ruslan Kuprieiev --- dvc/project.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/dvc/project.py b/dvc/project.py index 8050cc30d..811f68a08 100644 --- a/dvc/project.py +++ b/dvc/project.py @@ -236,11 +236,3 @@ class Project(object): for stage in self.stages(): outs += stage.outs return outs - - def pipelines(self): - pipelines = [] - for G in nx.weakly_connected_component_subgraphs(self.graph()): - pipeline = Pipeline(self, G) - pipelines.append(pipeline) - - return pipelines From 068f7428522f95c27d11c58f5067292ff9781bf9 Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Tue, 27 Feb 2018 03:51:20 +0200 Subject: [PATCH 7/7] dvc: use zc.lockfile instead of fasteners fasteners' InterProcessLock doesn't work within one process, thus it is not suitable for our dvc library. Signed-off-by: Ruslan Kuprieiev --- dvc/lock.py | 24 +++++++++++++++++++----- requirements.txt | 2 +- setup.py | 2 +- tests/test_lock.py | 22 ++++++++++++---------- 4 files changed, 33 insertions(+), 17 deletions(-) diff --git a/dvc/lock.py b/dvc/lock.py index 5063cd219..0fbff6160 100644 --- a/dvc/lock.py +++ b/dvc/lock.py @@ -1,5 +1,7 @@ import os -import fasteners +import time +import zc.lockfile + from dvc.exceptions import DvcException @@ -13,18 +15,30 @@ class Lock(object): def __init__(self, dvc_dir): self.lock_file = os.path.join(dvc_dir, self.LOCK_FILE) - self._lock = fasteners.InterProcessLock(self.lock_file) + self._lock = None @staticmethod def init(dvc_dir): return Lock(dvc_dir) - def lock(self): - if not self._lock.acquire(timeout=self.TIMEOUT): + def _do_lock(self): + try: + self._lock = zc.lockfile.LockFile(self.lock_file) + except zc.lockfile.LockError: raise LockError('Cannot perform the cmd since DVC is busy and locked. Please retry the cmd later.') + def lock(self): + try: + self._do_lock() + return + except LockError: + time.sleep(self.TIMEOUT) + + self._do_lock() + def unlock(self): - self._lock.release() + self._lock.close() + self._lock = None def __enter__(self): self.lock() diff --git a/requirements.txt b/requirements.txt index 8b540a23e..8e842dcc8 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,7 +1,7 @@ boto>=2.46.1 google-compute-engine>=2.4.1 #required by boto configparser>=3.5.0 -fasteners>=0.14.1 +zc.lockfile>=1.2.1 future>=0.16.0 google-cloud>=0.24.0 PyInstaller>=3.2.1 diff --git a/setup.py b/setup.py index 112468f5f..e81e01655 100644 --- a/setup.py +++ b/setup.py @@ -7,7 +7,7 @@ install_requires = [ "boto>=2.46.1", "google-compute-engine>=2.4.1", #required by boto "configparser>=3.5.0", - "fasteners>=0.14.1", + "zc.lockfile>=1.2.1", "future>=0.16.0", "google-cloud>=0.24.0", "colorama>=0.3.9", diff --git a/tests/test_lock.py b/tests/test_lock.py index f0963ff0a..edf3f1967 100644 --- a/tests/test_lock.py +++ b/tests/test_lock.py @@ -1,19 +1,21 @@ +from dvc.lock import LockError +from dvc.main import main from dvc.lock import Lock from tests.basic_env import TestDvc class TestLock(TestDvc): - def test_lock(self): - lock = Lock(self.dvc.dvc_dir) - lock.lock() - self.assertTrue(lock._lock.exists()) - self.assertTrue(lock._lock.acquired) - lock.unlock() - self.assertFalse(lock._lock.acquired) - def test_with(self): lock = Lock(self.dvc.dvc_dir) with lock: - self.assertTrue(lock._lock.acquired) - self.assertFalse(lock._lock.acquired) + with self.assertRaises(LockError): + lock2 = Lock(self.dvc.dvc_dir) + with lock2: + self.assertTrue(False) + + def test_cli(self): + lock = Lock(self.dvc.dvc_dir) + with lock: + ret = main(['add', self.FOO]) + self.assertEqual(ret, 1)