Merge branch 'master' of https://github.com/dmpetrov/dataversioncontrol

dvc.conf

@@ -2,17 +2,16 @@
 DataDir = data
 CacheDir = cache
 StateDir = state
-Cloud = google
+Cloud =
+StoragePath =
 
 # Log levels: Debug, Info, Warning and Error
 LogLevel = Info
 
 [AWS]
-AccessKeyId =
-SecretAccessKey =
-
 StoragePath =
 
+CredentialPath =
 
 Region = us-east-1
 Zone = us-east-1a
@@ -33,9 +32,6 @@ SecurityGroup = dvc-group
 
 
 [GC]
+StoragePath =
+
 ProjectName =
-StoragePath =
-
-
-[Data]
-StoragePath =

dvc/command/data_sync.py

@@ -85,7 +85,6 @@ class CmdDataSync(CmdBase):
 
     def _get_bucket_aws(self):
         """ get a bucket object, aws """
-
         conn = S3Connection(self.config.aws_access_key_id, self.config.aws_secret_access_key, host=self.config.aws_region_host)
         bucket_name = self.config.storage_bucket
         bucket = conn.lookup(bucket_name)
@@ -179,19 +178,19 @@ class CmdDataSync(CmdBase):
 
 
     def sync_from_cloud(self, item):
-        cloud = self.settings.config.get_cloud
+        cloud = self.settings.config.cloud
         assert cloud in ['amazon', 'google'], 'unknown cloud %s' % cloud
-        if self.settings.config.get_cloud == 'amazon':
+        if cloud == 'amazon':
             return self._sync_from_cloud_aws(item)
-        elif self.settings.config.get_cloud == 'google':
+        elif cloud == 'google':
             return self._sync_from_cloud_gcp(item)
 
     def sync_to_cloud(self, data_item):
-        cloud = self.settings.config.get_cloud
+        cloud = self.settings.config.cloud
         assert cloud in ['amazon', 'google'], 'unknown cloud %s' % cloud
-        if self.settings.config.get_cloud == 'amazon':
+        if cloud == 'amazon':
             return self._sync_to_cloud_aws(data_item)
-        elif self.settings.config.get_cloud == 'google':
+        elif cloud == 'google':
             return self._sync_to_cloud_gcp(data_item)
 
 

dvc/command/init.py

@@ -19,16 +19,15 @@ DataDir = {}
 CacheDir = {}
 StateDir = {}
 Cloud =
+StoragePath =
 
 # Log levels: Debug, Info, Warning and Error
 LogLevel = Info
 
 [AWS]
-AccessKeyId =
-SecretAccessKey =
-
 StoragePath = dvc/tutorial
 
+CredentialPath =
 
 Region = us-east-1
 Zone = us-east-1a
@@ -48,10 +47,8 @@ KeyName = dvc-key
 SecurityGroup = dvc-group
 
 [GC]
-ProjectName =
-
-[Data]
 StoragePath =
+ProjectName =
 '''
 
     def __init__(self, settings):

dvc/command/test.py

@@ -23,7 +23,7 @@ class CmdTest(CmdBase):
 
     def run(self):
 
-        cloud = self.settings._config.get_cloud
+        cloud = self.settings._config.cloud
         assert cloud in ['amazon', 'google'], 'unknown cloud: %s' % cloud
         if cloud == 'amazon':
             print 'TODO'

dvc/config.py

@@ -36,18 +36,27 @@ class ConfigI(object):
 
 class Config(ConfigI):
     CONFIG = 'dvc.conf'
-
-    def __init__(self, conf_file):
+    def __init__(self, conf_file, conf_pseudo_file=None):
+        """
+        Params:
+            conf_file (String): configuration file
+            conf_pseudo_file (String): for unit testing, something that supports readline; supersedes conf_file
+        """
         self._conf_file = conf_file
-        self._config = configparser.ConfigParser()
+        self._config = configparser.SafeConfigParser()
 
-        if not os.path.isfile(conf_file):
-            raise ConfigError('Config file "{}" does not exist'.format(conf_file))
-        self._config.read(conf_file)
+        if conf_pseudo_file is not None:
+            self._config.readfp(conf_pseudo_file)
+        else:
+            if not os.path.isfile(conf_file):
+                raise ConfigError('Config file "{}" does not exist'.format(conf_file))
+            self._config.read(conf_file)
 
         level = self._config['Global']['LogLevel']
         Logger.set_level(level)
 
+        self._aws_creds = None
+
         super(Config, self).__init__(self._config['Global']['DataDir'],
                                      self._config['Global']['CacheDir'],
                                      self._config['Global']['StateDir'])
@@ -59,11 +68,15 @@ class Config(ConfigI):
 
     @property
     def aws_access_key_id(self):
-        return self._config['AWS']['AccessKeyId']
+        if not self._aws_creds:
+            self._aws_creds = self.get_aws_credentials()
+        return self._aws_creds[0]
 
     @property
     def aws_secret_access_key(self):
-        return self._config['AWS']['SecretAccessKey']
+        if not self._aws_creds:
+            self._aws_creds = get_aws_credentials()
+        return self._aws_creds[1]
 
     @property
     def aws_region_host(self):
@@ -87,11 +100,11 @@ class Config(ConfigI):
         Precedence: Storage, then cloud specific
         """
 
-        path = self._config['Data'].get('StoragePath', None)
+        path = self._config['Global'].get('StoragePath', None)
         if path:
             return path
 
-        cloud = self.get_cloud
+        cloud = self.cloud
         assert cloud in ['amazon', 'google'], 'unknown cloud %s' % cloud
         if cloud == 'amazon':
             path = self._config['AWS'].get('StoragePath', None)
@@ -121,6 +134,101 @@ class Config(ConfigI):
         return self._config['GC']['ProjectName']
 
     @property
-    def get_cloud(self):
+    def cloud(self):
         """ get cloud choice: currently one of ['amazon', 'google'] """
-        return self._config['Global']['Cloud']
+        conf = self._config['Global'].get('Cloud', '')
+        if conf.lower() in ['amazon', 'aws']:
+            return 'amazon'
+        if conf.lower() in ['gc', 'google', 'google cloud', 'googlecloud']:
+            return 'google'
+        return conf
+
+
+    def get_aws_credentials(self):
+        """ gets aws credentials, looking in various places
+
+        Params:
+
+        Searches:
+        1 any override in dvc.conf [AWS] CredentialPath;
+        2 ~/.aws/credentials
+
+
+        Returns:
+            if successfully found, (access_key_id, secret)
+            None otherwise
+        """
+        default = os.path.expanduser('~/.aws/credentials')
+
+        paths = []
+        credpath = self._config['AWS'].get('CredentialPath', None)
+        if credpath is not None and len(credpath) > 0:
+            credpath = os.path.expanduser(credpath)
+            if os.path.isfile(credpath):
+                paths.append(credpath)
+            else:
+                Logger.warn('AWS CredentialPath "%s" not found; falling back to default "%s"' % (credpath, default))
+                paths.append(default)
+        else:
+            paths.append(default)
+
+        for path in paths:
+            cc = configparser.SafeConfigParser()
+            threw = False
+            try:
+                # use readfp(open( ... to aid mocking.
+                cc.readfp(open(path, 'r'))
+            except Exception as e:
+                threw = True
+            if not threw and 'default' in cc.keys():
+                access_key = cc['default'].get('aws_access_key_id', None)
+                secret = cc['default'].get('aws_secret_access_key', None)
+
+                if access_key is not None and secret is not None:
+                    return (access_key, secret)
+
+        return None
+
+
+    def sanity_check(self):
+        """ sanity check a config
+
+        check that we have a cloud and storagePath
+        if aws, check can read credentials
+        if google, check ProjectName
+
+        Returns:
+            (T,) if good
+            (F, issues) if bad
+        """
+        errors = []
+        for key in ['Cloud']:
+            if key.lower() not in self._config['Global'].keys() or len(self._config['Global'][key]) < 1:
+                errors.append('Please set %s in section Global in config file %s' % (key, self.file))
+
+        # now that a cloud is chosen, can check StoragePath
+        sp = self.storage_path
+        if sp is None or len(sp) == 0:
+            errors.append('Please set StoragePath = bucket/{optional path} in conf file "%s" '
+                           'either in Global or a cloud specific section' % self.CONFIG)
+
+        cloud = self.cloud
+        if cloud == '':
+            # not set; already complained above
+            pass
+        elif cloud == 'amazon':
+            creds = self.get_aws_credentials()
+            if creds is None:
+                errors.append('can\'t find aws credentials.  TODO')
+            self._aws_creds = creds
+        elif cloud == 'google':
+            project = self._config['GC'].get('ProjectName', None)
+            if project is None or len(project) < 1:
+                errors.append('can\'t read google cloud project name.  Please set ProjectName in section GC.  TODO HOW FIND')
+        else:
+            errors.append('unknown Cloud %s' % cloud)
+
+        return (len(errors) == 0, errors)
+
+
+

dvc/runtime.py

@@ -29,6 +29,13 @@ class Runtime(object):
 
             if parse_config:
                 runtime_config = Config(Runtime.conf_file_path(runtime_git.git_dir))
+
+                good = runtime_config.sanity_check()
+                if good[0] == False:
+                    Logger.error('config \'%s\' is not correctly setup.  Please fix:' % Runtime.CONFIG)
+                    for e in good[1]:
+                        Logger.error('    ' + e)
+                    sys.exit(-1)
             else:
                 runtime_config = ConfigI()
 

requirements.txt

@@ -9,6 +9,7 @@ decorator==4.0.11
 dill==0.2.6
 enum34==1.1.6
 fasteners==0.14.1
+funcsigs==1.0.2
 future==0.16.0
 futures==3.0.5
 gapic-google-cloud-datastore-v1==0.15.3
@@ -48,6 +49,7 @@ httplib2==0.10.3
 ipython==5.3.0
 ipython-genutils==0.2.0
 macholib==1.8
+mock==2.0.0
 modulegraph==0.14
 monotonic==1.3
 nose==1.3.7
@@ -55,6 +57,7 @@ oauth2client==3.0.0
 packaging==16.8
 pathlib==1.0.1
 pathlib2==2.2.1
+pbr==3.0.0
 pexpect==4.2.1
 pickleshare==0.7.4
 ply==3.8

tests/test_config.py

@@ -0,0 +1,129 @@
+import io
+import os
+import shutil
+import StringIO
+import traceback
+from unittest import TestCase
+import mock
+
+
+from dvc.path.path import Path
+from dvc.system import System
+from dvc.config import Config
+
+
+class TestConfigTest(TestCase):
+    def setUp(self):
+        pass
+
+    def storage_path_hierarchy_test(self):
+        """ StoragePath should be read first from global and then from cloud section """
+
+        c = ("[Global]",
+             "LogLevel =",
+             "DataDir = ",
+             "CacheDir = ",
+             "StateDir = ",
+             "Cloud = amazon",
+             "StoragePath = globalsb/global_storage_path",
+             "[AWS]",
+             "StoragePath = awssb/aws_storage_path",
+             "CredentialPath =",
+             "[GC]",
+             "StoragePath = googlesb/google_storage_path"
+           )
+        s = StringIO.StringIO('\n'.join(c))
+        conf = Config(s, conf_pseudo_file = s)
+
+        self.assertEqual(conf.cloud, 'amazon')
+        self.assertEqual(conf.storage_bucket, 'globalsb')
+        self.assertEqual(conf.storage_prefix, 'global_storage_path')
+
+        c = ("[Global]",
+             "LogLevel =",
+             "DataDir = ",
+             "CacheDir = ",
+             "StateDir = ",
+             "Cloud = amazon",
+             "[AWS]",
+             "StoragePath = awssb/aws_storage_path",
+             "CredentialPath =",
+             "[GC]",
+             "StoragePath = googlesb/google_storage_path"
+           )
+        s = StringIO.StringIO('\n'.join(c))
+        conf = Config(s, conf_pseudo_file=s)
+
+        self.assertEqual(conf.cloud, 'amazon')
+        self.assertEqual(conf.storage_bucket, 'awssb')
+        self.assertEqual(conf.storage_prefix, 'aws_storage_path')
+
+
+    def mocked_open_aws_default_credentials(klass, file, asdf):
+        """ mocked open for ~/.aws/credentials """
+        if file == os.path.expanduser('~/.aws/credentials'):
+            creds0 = ("[default]",
+                      "aws_access_key_id = default_access_id",
+                      "aws_secret_access_key = default_sekret")
+            return io.BytesIO('\n'.join(creds0))
+
+        creds1 = ("[default]",
+                  "aws_access_key_id = override_access_id",
+                  "aws_secret_access_key = override_sekret")
+
+        return io.BytesIO('\n'.join(creds1))
+
+    def aws_credentials_default_test(self):
+        """ in absence of [AWS] -> CredentialPath, aws creds should be read from ~/.aws/credentials """
+
+        default_path = os.path.expanduser('~/.aws/credentials')
+        c = ("[Global]",
+             "LogLevel =",
+             "DataDir = ",
+             "CacheDir = ",
+             "StateDir = ",
+             "Cloud = amazon",
+             "[AWS]",
+             "StoragePath = awssb/aws_storage_path",
+             "CredentialPath =",
+           )
+        s = StringIO.StringIO('\n'.join(c))
+
+        patcher = mock.patch('__builtin__.open', side_effect=self.mocked_open_aws_default_credentials)
+        patcher.start()
+        conf = Config(s, conf_pseudo_file=s)
+        aws_creds = conf.get_aws_credentials()
+        patcher.stop()
+
+        self.assertEqual(aws_creds[0], 'default_access_id')
+        self.assertEqual(aws_creds[1], 'default_sekret')
+
+
+    def mocked_isfile(file):
+        """ tell os some_credential_path exists """
+        return file == 'some_credential_path'
+
+    @mock.patch('dvc.config.os.path.isfile', side_effect = mocked_isfile)
+    def aws_credentials_specified_test(self, isfile_function):
+        """ in presence of [AWS] -> CredentialPath, use those credentials """
+
+        c = ("[Global]",
+             "LogLevel =",
+             "DataDir = ",
+             "CacheDir = ",
+             "StateDir = ",
+             "Cloud = amazon",
+             "[AWS]",
+             "StoragePath = awssb/aws_storage_path",
+             "CredentialPath = some_credential_path",
+           )
+        s = StringIO.StringIO('\n'.join(c))
+
+        patcher = mock.patch('__builtin__.open', side_effect=self.mocked_open_aws_default_credentials)
+        patcher.start()
+        conf = Config(s, conf_pseudo_file=s)
+        aws_creds = conf.get_aws_credentials()
+        patcher.stop()
+
+        self.assertEqual(aws_creds[0], 'override_access_id')
+        self.assertEqual(aws_creds[1], 'override_sekret')

tests/test_executor.py

@@ -18,6 +18,8 @@ class TestExecutor(TestCase):
 
     def tearDown(self):
         shutil.rmtree(self.test_dir)
+        if os.path.exists('myfile.py'):
+            os.remove('myfile.py')
 
     def test_suppress_output(self):
         output = Executor.exec_cmd_only_success(['python', self.code_file])