Merge pull request #98 from dataversioncontrol/sync_subfolder_fix

Sync subfolder fix

dvc/command/data_sync.py

@@ -7,13 +7,14 @@ from dvc.runtime import Runtime
 from dvc.system import System
 from dvc.data_cloud import DataCloud
 
-#FIXME add cmdline argument and config option for POOL_SIZE
 POOL_SIZE = 4
 
+
 class DataSyncError(DvcException):
     def __init__(self, msg):
         super(DataSyncError, self).__init__('Data sync error: {}'.format(msg))
 
+
 class CmdDataSync(CmdBase):
     def __init__(self, settings):
         super(CmdDataSync, self).__init__(settings)
@@ -30,6 +31,9 @@ class CmdDataSync(CmdBase):
             pool = ThreadPool(processes=POOL_SIZE)
             targets = []
 
+            if len(self.parsed_args.targets) == 0:
+                raise DataSyncError('Sync target is not specified')
+
             for target in self.parsed_args.targets:
                 if System.islink(target):
                     targets.append(target)

dvc/command/import_file.py

@@ -14,6 +14,7 @@ from dvc.system import System
 from dvc.command.data_sync import POOL_SIZE
 from dvc.progress import Progress
 
+
 class ImportFileError(DvcException):
     def __init__(self, msg):
         DvcException.__init__(self, 'Import file: {}'.format(msg))
@@ -37,6 +38,8 @@ class CmdImportFile(CmdBase):
         targets = []
         with DvcLock(self.is_locker, self.git):
             output = self.parsed_args.output
+            self.verify_output(output, self.parsed_args.input)
+
             for input in self.parsed_args.input:
                 if not os.path.isdir(input):
                     targets.append((input, output))
@@ -60,6 +63,20 @@ class CmdImportFile(CmdBase):
             self.commit_if_needed(message)
         pass
 
+    @staticmethod
+    def verify_output(output, input):
+        if CmdImportFile.is_dir_path(output) and not os.path.isdir(output):
+            raise ImportFileError(u'output directory {} does not exist'.format(output))
+
+        if len(input) > 1 and not os.path.isdir(output):
+            msg = u'output {} has to be directory for multiple file import'
+            raise ImportFileError(msg.format(output))
+        pass
+
+    @staticmethod
+    def is_dir_path(output):
+        return len(output) > 0 and output[-1] == os.path.sep
+
     def import_and_commit_if_needed(self, input, output, lock=False, check_if_ready=True):
         if check_if_ready and not self.no_git_actions and not self.git.is_ready_to_go():
             return 1
@@ -213,4 +230,4 @@ class CmdImportFile(CmdBase):
 
 
 if __name__ == '__main__':
-    Runtime.run(CmdDataImport)
+    Runtime.run(CmdImportFile)

dvc/command/remove.py

@@ -6,12 +6,12 @@ from dvc.path.data_item import DataItemError
 from dvc.runtime import Runtime
 
 
-class CmdDataRemove(Traverse):
+class CmdRemove(Traverse):
     def __init__(self, settings):
-        super(CmdDataRemove, self).__init__(settings, "remove")
+        super(CmdRemove, self).__init__(settings, "remove")
 
     def define_args(self, parser):
-        super(CmdDataRemove, self).define_args(parser)
+        super(CmdRemove, self).define_args(parser)
         parser.add_argument('-r', '--recursive', action='store_true', help='Remove directory recursively.')
         parser.add_argument('-c', '--keep-in-cache', action='store_false', default=False,
                             help='Do not remove data from cache.')
@@ -86,4 +86,4 @@ class CmdDataRemove(Traverse):
 
 
 if __name__ == '__main__':
-    Runtime.run(CmdDataRemove)
+    Runtime.run(CmdRemove)

dvc/data_cloud.py

@@ -1,20 +1,19 @@
 import base64
 import hashlib
 import os
+import threading
 import configparser
 import shutil
 
 from boto.s3.connection import S3Connection
 from google.cloud import storage as gc
 
-from dvc.command.base import CmdBase, DvcLock
 from dvc.logger import Logger
 from dvc.exceptions import DvcException
-from dvc.runtime import Runtime
-from dvc.system import System
 from dvc.config import ConfigError
 from dvc.settings import Settings
 
+
 class DataCloudError(DvcException):
     def __init__(self, msg):
         super(DataCloudError, self).__init__('Data sync error: {}'.format(msg))
@@ -47,6 +46,7 @@ class DataCloudBase(object):
         self._settings = settings
         self._config = config
         self._cloud_config = cloud_config
+        self._lock = threading.Lock()
 
     @property
     def storage_path(self):
@@ -97,8 +97,25 @@ class DataCloudBase(object):
         if os.path.isfile(item.resolved_cache.dvc):
             self.sync_to_cloud(item)
         else:
+            self.create_directory(fname, item)
             self.sync_from_cloud(item)
 
+    def create_directory(self, fname, item):
+        self._lock.acquire()
+        try:
+            dir = os.path.dirname(item.cache.relative)
+            if not os.path.exists(dir):
+                Logger.debug(u'Creating directory {}'.format(dir))
+                try:
+                    os.makedirs(dir)
+                except OSError as ex:
+                    raise DataCloudError(u'Cannot create directory {}: {}'.format(dir, ex))
+            elif not os.path.isdir(dir):
+                msg = u'File {} cannot be synced because {} is not a directory'
+                raise DataCloudError(msg.format(fname, dir))
+        finally:
+            self._lock.release()
+
     def remove_from_cloud(self, item):
         pass
 
@@ -116,6 +133,7 @@ class DataCloudLOCAL(DataCloudBase):
         Logger.debug('rm from cloud ' + item.resolved_cache.dvc)
         os.remove(item.resolved_cache.dvc)
 
+
 class DataCloudAWS(DataCloudBase):
     """ DataCloud class for Amazon Web Services """
     @property

dvc/git_wrapper.py

@@ -70,6 +70,9 @@ class GitWrapperI(object):
 
         return result
 
+    def commit_all_changes_and_log_status(self, message):
+        pass
+
 
 class GitWrapper(GitWrapperI):
     def __init__(self):

dvc/main.py

@@ -10,7 +10,7 @@ import sys
 
 from dvc.runtime import Runtime
 from dvc.command.init import CmdInit
-from dvc.command.remove import CmdDataRemove
+from dvc.command.remove import CmdRemove
 from dvc.command.run import CmdRun
 from dvc.command.repro import CmdRepro
 from dvc.command.data_sync import CmdDataSync
@@ -64,7 +64,7 @@ def main():
     elif cmd == 'import':
         Runtime.run(CmdImportFile)
     elif cmd == 'remove':
-        Runtime.run(CmdDataRemove)
+        Runtime.run(CmdRemove)
     elif cmd == 'lock':
         Runtime.run(CmdLock)
     elif cmd == 'cloud-run':

dvc/system.py

@@ -82,9 +82,7 @@ class System(object):
     def get_long_path(path):
         """Convert short path to a full path. It is needed for Windows."""
         if System.is_unix():
-            print('======================11111111111111111111')
             return path
-        print('======================222222222222222222222')
 
         buffer = create_unicode_buffer(System.LONG_PATH_BUFFER_SIZE)
         get_long_path_name = windll.kernel32.GetLongPathNameW

tests/test_cmd_gc.py

@@ -1,4 +1,5 @@
-import glob, os
+import glob
+import os
 
 from dvc.command.gc import CmdGC
 from dvc.config import ConfigI

tests/test_cmd_remove.py

@@ -1,15 +1,15 @@
 import os
 
-from dvc.command.remove import CmdDataRemove
+from dvc.command.remove import CmdRemove
 from tests.basic_env import DirHierarchyEnvironment
 
 
-class TestCmdDataRemove(DirHierarchyEnvironment):
+class TestCmdRemove(DirHierarchyEnvironment):
     def setUp(self):
         DirHierarchyEnvironment.init_environment(self)
 
     def test_file_by_file_removal(self):
-        cmd = CmdDataRemove(self.settings)
+        cmd = CmdRemove(self.settings)
         cmd.parsed_args.keep_in_cloud = True
 
         dir1_dvc_name = os.path.join('data', self.dir1)
@@ -19,7 +19,7 @@ class TestCmdDataRemove(DirHierarchyEnvironment):
         self.assertFalse(os.path.exists(dir1_dvc_name))
 
     def test_remove_deep_dir(self):
-        cmd = CmdDataRemove(self.settings)
+        cmd = CmdRemove(self.settings)
         cmd.parsed_args.keep_in_cloud = True
         cmd.parsed_args.recursive = True
 
@@ -30,7 +30,7 @@ class TestCmdDataRemove(DirHierarchyEnvironment):
         self.assertFalse(os.path.exists(dir1_dvc_name))
 
     def test_not_recursive_removal(self):
-        cmd = CmdDataRemove(self.settings)
+        cmd = CmdRemove(self.settings)
         cmd.parsed_args.keep_in_cloud = True
         cmd.parsed_args.recursive = False
 
@@ -41,7 +41,7 @@ class TestCmdDataRemove(DirHierarchyEnvironment):
         pass
 
     def test_data_dir_removal(self):
-        cmd = CmdDataRemove(self.settings)
+        cmd = CmdRemove(self.settings)
         cmd.parsed_args.keep_in_cloud = True
         cmd.parsed_args.recursive = True
 
@@ -57,7 +57,7 @@ class TestRemoveDataItem(DirHierarchyEnvironment):
         DirHierarchyEnvironment.init_environment(self)
 
     def test_remove_data_instance(self):
-        cmd = CmdDataRemove(self.settings)
+        cmd = CmdRemove(self.settings)
         cmd.parsed_args.keep_in_cloud = True
 
         self.assertTrue(os.path.isfile(self.file1))
@@ -70,7 +70,7 @@ class TestRemoveDataItem(DirHierarchyEnvironment):
         self.assertFalse(os.path.exists(self.state1))
 
     def test_keep_cache(self):
-        cmd = CmdDataRemove(self.settings)
+        cmd = CmdRemove(self.settings)
         cmd.parsed_args.keep_in_cloud = True
         cmd.parsed_args.keep_in_cache = True
 
@@ -84,7 +84,7 @@ class TestRemoveDataItem(DirHierarchyEnvironment):
         self.assertFalse(os.path.exists(self.state1))
 
     def test_remove_data_instance_without_cache(self):
-        cmd = CmdDataRemove(self.settings)
+        cmd = CmdRemove(self.settings)
         cmd.parsed_args.keep_in_cloud = True
 
         self.assertTrue(os.path.isfile(self.file6))
@@ -100,35 +100,8 @@ class TestRemoveEndToEnd(DirHierarchyEnvironment):
     def setUp(self):
         DirHierarchyEnvironment.init_environment(self)
 
-    # def test_end_to_end_with_an_error(self):
-    #     cmd = CmdDataRemove(self.settings)
-    #     cmd.parsed_args.keep_in_cloud = True
-    #
-    #     dir11_full = self.dir11
-    #     dir2_full = self.dir2
-    #     file6_full = self.file6
-    #
-    #     cmd.parsed_args.target = [dir11_full, self.file5, file6_full]
-    #     cmd.parsed_args.recursive = True
-    #
-    #     cmd.parsed_args.skip_git_actions = True
-    #
-    #     self.assertTrue(os.path.exists(dir11_full))
-    #     self.assertTrue(os.path.exists(self.file5))
-    #     self.assertTrue(os.path.exists(dir2_full))
-    #     self.assertTrue(os.path.exists(self.file1))
-    #     self.assertTrue(os.path.exists(self.file6))
-    #
-    #     self.assertFalse(cmd.remove_all_targets())
-    #
-    #     self.assertFalse(os.path.exists(dir11_full))
-    #     self.assertFalse(os.path.exists(self.file5))
-    #     self.assertTrue(os.path.exists(dir2_full))
-    #     self.assertTrue(os.path.exists(self.file1))
-    #     self.assertTrue(os.path.exists(self.file6))
-
     def test_end_to_end_with_no_error(self):
-        cmd = CmdDataRemove(self.settings)
+        cmd = CmdRemove(self.settings)
         cmd.parsed_args.keep_in_cloud = True
 
         dir11_full = os.path.join('data', self.dir11)
@@ -152,7 +125,7 @@ class TestRemoveEndToEnd(DirHierarchyEnvironment):
         self.assertTrue(os.path.exists(self.file1))
 
     def test_run(self):
-        cmd = CmdDataRemove(self.settings)
+        cmd = CmdRemove(self.settings)
         cmd.parsed_args.keep_in_cloud = True
 
         dir11_full = os.path.join('data', self.dir11)
@@ -181,7 +154,7 @@ class TestRemoveEndToEndWithAnError(DirHierarchyEnvironment):
         DirHierarchyEnvironment.init_environment(self)
 
     def test(self):
-        cmd = CmdDataRemove(self.settings)
+        cmd = CmdRemove(self.settings)
         cmd.parsed_args.keep_in_cloud = True
 
         dir11_full = os.path.join(self._config.data_dir, self.dir11)

tests/test_config.py

@@ -1,7 +1,4 @@
-import io
 import os
-import shutil
-import traceback
 from unittest import TestCase
 import mock
 
@@ -18,8 +15,6 @@ except ImportError:
     # Python 3
     builtin_module_name = 'builtins'
 
-from dvc.path.path import Path
-from dvc.system import System
 from dvc.config import Config
 from dvc.data_cloud import DataCloud
 from dvc.settings import Settings

tests/test_import.py

@@ -0,0 +1,118 @@
+import os
+import tempfile
+from copy import copy
+
+from dvc.command.import_file import CmdImportFile, ImportFileError
+from tests.basic_env import DirHierarchyEnvironment
+
+
+class TestCmdRemove(DirHierarchyEnvironment):
+    def setUp(self):
+        DirHierarchyEnvironment.init_environment(self)
+
+    def test_import_file_to_dir(self):
+        with tempfile.NamedTemporaryFile(mode='w') as temp:
+            content = 'Some data'
+            temp.write(content)
+            temp.flush()
+
+            dir = os.path.join('data', self.dir1)
+            settings = copy(self.settings)
+            settings.set_args([temp.name, dir])
+            cmd = CmdImportFile(settings)
+            cmd.run()
+
+            dvc_name = os.path.join(dir, os.path.basename(temp.name))
+            data_item = self.path_factory.existing_data_item(dvc_name)
+
+            self.assertTrue(os.path.exists(data_item.data.relative))
+            self.assertTrue(os.path.islink(data_item.data.relative))
+            self.assertEqual(open(data_item.data.relative).read(), content)
+
+            self.assertTrue(os.path.exists(data_item.cache.relative))
+            self.assertEqual(open(data_item.cache.relative).read(), content)
+
+            self.assertTrue(os.path.exists(data_item.state.relative))
+        pass
+
+    def test_import_file_to_file(self):
+        with tempfile.NamedTemporaryFile(mode='w') as temp:
+            content = 'Some data'
+            temp.write(content)
+            temp.flush()
+
+            target_file_name = 'targ.txt'
+            target_file = os.path.join('data', self.dir1, target_file_name)
+            settings = copy(self.settings)
+            settings.set_args([temp.name, target_file])
+            cmd = CmdImportFile(settings)
+            cmd.run()
+
+            self.assertTrue(os.path.exists(target_file))
+            self.assertTrue(open(target_file).read(), content)
+        pass
+
+    def test_import_to_existing_dir(self):
+        with tempfile.NamedTemporaryFile(mode='w') as temp:
+            content = 'Some data'
+            temp.write(content)
+            temp.flush()
+
+            dir = os.path.join('data', self.dir11)
+            settings = copy(self.settings)
+            settings.set_args([temp.name, dir])
+            cmd = CmdImportFile(settings)
+            cmd.run()
+
+            dvc_name = os.path.join(dir, os.path.basename(temp.name))
+
+            self.assertTrue(os.path.exists(dvc_name))
+            self.assertTrue(open(dvc_name).read(), content)
+        pass
+
+    def test_import_to_not_existing_dir(self):
+        with tempfile.NamedTemporaryFile(mode='w') as temp:
+            content = 'Some data'
+            temp.write(content)
+            temp.flush()
+
+            dir = os.path.join('data', self.dir1, 'not_existing_dir/')
+            settings = copy(self.settings)
+            settings.set_args([temp.name, dir])
+            cmd = CmdImportFile(settings)
+
+            with self.assertRaises(ImportFileError):
+                cmd.run()
+
+            dvc_name = os.path.join(dir, os.path.basename(temp.name))
+
+            self.assertFalse(os.path.exists(dir))
+            self.assertFalse(os.path.exists(dvc_name))
+        pass
+
+    def test_multiple_import(self):
+        with tempfile.NamedTemporaryFile(mode='w') as temp1,\
+                tempfile.NamedTemporaryFile(mode='w') as temp2:
+            content = 'Some data'
+
+            temp1.write(content)
+            temp1.flush()
+
+            temp2.write(content)
+            temp2.flush()
+
+            dir = os.path.join('data', self.dir11)
+            settings = copy(self.settings)
+            settings.set_args([temp1.name, temp2.name, dir])
+            cmd = CmdImportFile(settings)
+            cmd.run()
+
+            dvc_name1 = os.path.join(dir, os.path.basename(temp1.name))
+            dvc_name2 = os.path.join(dir, os.path.basename(temp2.name))
+
+            self.assertTrue(os.path.exists(dvc_name1))
+            self.assertTrue(open(dvc_name1).read(), content)
+
+            self.assertTrue(os.path.exists(dvc_name2))
+            self.assertTrue(open(dvc_name2).read(), content)
+        pass

tests/test_repro.py

@@ -1,10 +1,9 @@
 import os
 
-from dvc.command.remove import CmdDataRemove
+from dvc.command.remove import CmdRemove
 from dvc.command.repro import CmdRepro
 from dvc.command.run import CmdRun
 from dvc.executor import Executor
-from dvc.logger import Logger
 from tests.test_cmd_run import RunBasicTest
 
 
@@ -94,7 +93,7 @@ class ReproChangedDependency(ReproBasicEnv):
 
     def recreate_file1(self):
         self.settings._args = [self.file_name1, '--keep-in-cloud']
-        CmdDataRemove(self.settings).run()
+        CmdRemove(self.settings).run()
 
         file1_code_file = 'file1_2.py'
         self.create_file_and_commit(file1_code_file, 'print("Goodbye")' + os.linesep + 'print("Jack")')