Repository: dataversioncontrol/dvc
Pull request #630 (2018-04-08 01:37:48)

Improve url testing for data cloud


Patch:
diff --git a/dvc/__init__.py b/dvc/__init__.py
index 1a2185000..f9f688728 100644
--- a/dvc/__init__.py
+++ b/dvc/__init__.py
@@ -27,7 +27,7 @@ if os.path.exists(os.path.join(HOMEPATH, 'setup.py')):
             # Write a helper file, that will be installed with the package
             # and will provide a true version of the installed dvc
             with open(VERSIONPATH, 'w+') as fd:
-                fd.write('# AUTOGENERATED by dvc/__init__.py')
+                fd.write('# AUTOGENERATED by dvc/__init__.py\n')
                 fd.write('version = "{}"'.format(__version__))
         except:
             pass
diff --git a/dvc/cloud/aws.py b/dvc/cloud/aws.py
index bb1c517b1..e14ed71c4 100644
--- a/dvc/cloud/aws.py
+++ b/dvc/cloud/aws.py
@@ -47,6 +47,8 @@ def create_cb(name, offset=0, multipart_total=None):
 
 class DataCloudAWS(DataCloudBase):
     """ DataCloud class for Amazon Web Services """
+    REGEX = r'^s3://(?P<path>.*)$'
+
     def __init__(self, cloud_settings):
         super(DataCloudAWS, self).__init__(cloud_settings)
         self._aws_creds = AWSCredentials(cloud_settings.cloud_config)
diff --git a/dvc/cloud/base.py b/dvc/cloud/base.py
index 8f2b3d02d..cce7625b9 100644
--- a/dvc/cloud/base.py
+++ b/dvc/cloud/base.py
@@ -33,9 +33,33 @@ class DataCloudError(DvcException):
 
 class DataCloudBase(object):
     """ Base class for DataCloud """
+
+    REGEX = r''
+
     def __init__(self, cloud_settings):
         self._cloud_settings = cloud_settings
 
+    @property
+    def url(self):
+        config = self._cloud_settings.cloud_config
+        return config.get(Config.SECTION_REMOTE_URL, None)
+
+    @classmethod
+    def match(cls, url):
+        return re.match(cls.REGEX, url)
+
+    def group(self, group):
+        return self.match(self.url).group(group)
+
+    @property
+    def path(self):
+        return self.group('path')
+
+    @classmethod
+    def supported(cls, url):
+        return cls.match(url) != None
+
+    # backward compatibility
     @property
     def storage_path(self):
         """ get storage path
@@ -46,14 +70,12 @@ class DataCloudBase(object):
         if self._cloud_settings.global_storage_path:
             return self._cloud_settings.global_storage_path
 
-        config = self._cloud_settings.cloud_config
-        url = config.get(Config.SECTION_REMOTE_URL, None)
-        if not url:
-            path = config.get(Config.SECTION_CORE_STORAGEPATH, None)
+        if not self.url:
+            path = self._cloud_settings.cloud_config.get(Config.SECTION_CORE_STORAGEPATH, None)
             if path:
                 Logger.warn('Using obsoleted config format. Consider updating.')
         else:
-            path = re.match(Config.SECTION_REMOTE_URL_REGEX, url).group('storagepath')
+            path = self.path
 
         return path
 
diff --git a/dvc/cloud/data_cloud.py b/dvc/cloud/data_cloud.py
index 5770f75a8..480cf8a36 100644
--- a/dvc/cloud/data_cloud.py
+++ b/dvc/cloud/data_cloud.py
@@ -4,9 +4,8 @@ from multiprocessing.pool import ThreadPool
 from dvc.cloud.instance_manager import CloudSettings
 from dvc.logger import Logger
 from dvc.exceptions import DvcException
-from dvc.config import ConfigError
+from dvc.config import Config, ConfigError
 from dvc.utils import map_progress
-from dvc.config import Config
 
 from dvc.cloud.aws import DataCloudAWS
 from dvc.cloud.gcp import DataCloudGCP
@@ -23,12 +22,6 @@ class DataCloud(object):
         'local' : DataCloudLOCAL,
     }
 
-    SCHEME_MAP = {
-        's3'    : 'aws',
-        'gs'    : 'gcp',
-        None    : 'local',
-    }
-
     def __init__(self, cache, config):
         self._cache = cache
         self._config = config
@@ -45,6 +38,13 @@ class DataCloud(object):
 
         self._cloud = self._init_remote(remote)
 
+    @staticmethod
+    def supported(url):
+        for cloud in DataCloud.CLOUD_MAP.values():
+            if cloud.supported(url):
+                return cloud
+        return None
+
     def _init_remote(self, remote):
         section = Config.SECTION_REMOTE_FMT.format(remote)
         cloud_config = self._config.get(section, None)
@@ -52,28 +52,25 @@ class DataCloud(object):
             raise ConfigError("Can't find remote section '{}' in config".format(section))
 
         url = cloud_config[Config.SECTION_REMOTE_URL]
-        res = re.match(Config.SECTION_REMOTE_URL_REGEX, url)
-        if not res:
-            raise ConfigError("Unknown format of url '{}'".format(url))
-
-        scheme = res.group('scheme')
-        cloud_type = self.SCHEME_MAP.get(scheme, None)
+        cloud_type = self.supported(url)
         if not cloud_type:
-            raise ConfigError("Unsupported scheme '{}' in '{}'".format(scheme, url))
+            raise ConfigError("Unsupported url '{}'".format(url))
 
         return self._init_cloud(self._cache, cloud_config, cloud_type)
 
     def __init__compat(self):
-        cloud_type = self._config[Config.SECTION_CORE].get(Config.SECTION_CORE_CLOUD, '').strip().lower()
-        if cloud_type == '':
+        cloud_name = self._config[Config.SECTION_CORE].get(Config.SECTION_CORE_CLOUD, '').strip().lower()
+        if cloud_name == '':
             self._cloud = None
             return
-        elif cloud_type not in self.CLOUD_MAP.keys():
-            raise ConfigError('Wrong cloud type %s specified' % cloud_type)
 
-        cloud_config = self._config.get(cloud_type, None)
+        cloud_type = self.CLOUD_MAP.get(cloud_name, None)
+        if not cloud_type:
+            raise ConfigError('Wrong cloud type %s specified' % cloud_name)
+
+        cloud_config = self._config.get(cloud_name, None)
         if not cloud_config:
-            raise ConfigError('Can\'t find cloud section \'[%s]\' in config' % cloud_type)
+            raise ConfigError('Can\'t find cloud section \'[%s]\' in config' % cloud_name)
 
         return self._init_cloud(self._cache, cloud_config, cloud_type)
 
@@ -82,7 +79,7 @@ class DataCloud(object):
                                                  self._config,
                                                  cloud_config)
 
-        cloud = self.CLOUD_MAP[cloud_type](cloud_settings)
+        cloud = cloud_type(cloud_settings)
         cloud.sanity_check()
         return cloud
 
diff --git a/dvc/cloud/gcp.py b/dvc/cloud/gcp.py
index 50f5e2aac..28d345dfe 100644
--- a/dvc/cloud/gcp.py
+++ b/dvc/cloud/gcp.py
@@ -12,6 +12,8 @@ from dvc.progress import progress
 
 class DataCloudGCP(DataCloudBase):
     """ DataCloud class for Google Cloud Platform """
+    REGEX = r'^gs://(?P<path>.*)$'
+
     @property
     def gc_project_name(self):
         """
diff --git a/dvc/cloud/local.py b/dvc/cloud/local.py
index 880937129..08d3b7135 100644
--- a/dvc/cloud/local.py
+++ b/dvc/cloud/local.py
@@ -20,6 +20,8 @@ class DataCloudLOCAL(DataCloudBase):
     """
     Driver for local storage.
     """
+    REGEX = r'^(?P<path>(/+|.:\\+).*)$'
+
     def cache_file_key(self, path):
         return os.path.relpath(os.path.abspath(path), self._cloud_settings.cache.cache_dir)
 
diff --git a/dvc/config.py b/dvc/config.py
index 99b8bdf96..156c77c30 100644
--- a/dvc/config.py
+++ b/dvc/config.py
@@ -14,6 +14,11 @@ class ConfigError(DvcException):
         super(ConfigError, self).__init__('Config file error', ex)
 
 
+def supported_url(url):
+    from dvc.cloud.data_cloud import DataCloud
+    return DataCloud.supported(url) != None
+
+
 class Config(object):
     CONFIG = 'config'
 
@@ -28,7 +33,7 @@ class Config(object):
     SECTION_CORE_STORAGEPATH = 'storagepath'
 
     SECTION_CORE_SCHEMA = {
-        schema.Optional(SECTION_CORE_LOGLEVEL, default='info'): SECTION_CORE_LOGLEVEL_SCHEMA,
+        schema.Optional(SECTION_CORE_LOGLEVEL, default='info'): schema.And(str, schema.Use(str.lower), SECTION_CORE_LOGLEVEL_SCHEMA),
         schema.Optional(SECTION_CORE_REMOTE, default=''): schema.And(str, schema.Use(str.lower)),
 
         # backward compatibility
@@ -69,9 +74,8 @@ class Config(object):
     SECTION_REMOTE_REGEX = r'^\s*remote\s*"(.*)"\s*$'
     SECTION_REMOTE_FMT = 'remote "{}"'
     SECTION_REMOTE_URL = 'url'
-    SECTION_REMOTE_URL_REGEX = r'^\s*(((?P<scheme>s3|gs|..+):/*)|/*|.\\*)?(?P<storagepath>.*)\s*$'
     SECTION_REMOTE_SCHEMA = {
-        SECTION_REMOTE_URL: schema.Regex(SECTION_REMOTE_URL_REGEX),
+        SECTION_REMOTE_URL: supported_url,
         schema.Optional(SECTION_AWS_REGION): str,
         schema.Optional(SECTION_AWS_PROFILE, default='default'): str,
         schema.Optional(SECTION_AWS_CREDENTIALPATH, default = ''): str,
diff --git a/dvc/logger.py b/dvc/logger.py
index 40bcad2f4..2ecd6246a 100644
--- a/dvc/logger.py
+++ b/dvc/logger.py
@@ -3,7 +3,6 @@ import logging
 import colorama
 import traceback
 
-from dvc.config import Config
 
 colorama.init()
 
@@ -25,13 +24,12 @@ class Logger(object):
         'error': colorama.Fore.RED
     }
 
-    def __init__(self, config=None):
-        if config:
-            level = config[Config.SECTION_CORE].get('LogLevel', None)
-            Logger.set_level(level)
+    def __init__(self, loglevel=None):
+        if loglevel:
+            Logger.set_level(loglevel)
 
     @staticmethod
-    def init(config=None):
+    def init():
         sh = logging.StreamHandler(sys.stdout)
         sh.setFormatter(logging.Formatter(Logger.FMT))
         sh.setLevel(logging.DEBUG)
diff --git a/dvc/project.py b/dvc/project.py
index e8ecdbb8e..71f943f84 100644
--- a/dvc/project.py
+++ b/dvc/project.py
@@ -40,7 +40,7 @@ class Project(object):
         self.cache = Cache(self.dvc_dir)
         self.state = State(self.root_dir, self.dvc_dir)
         self.config = Config(self.dvc_dir)
-        self.logger = Logger(self.config._config)
+        self.logger = Logger(self.config._config[Config.SECTION_CORE].get(Config.SECTION_CORE_LOGLEVEL, None))
         self.cloud = DataCloud(self.cache, self.config._config)
 
     @staticmethod


Test patch:
diff --git a/tests/test_logger.py b/tests/test_logger.py
index cdb5a1919..8959bc462 100644
--- a/tests/test_logger.py
+++ b/tests/test_logger.py
@@ -9,12 +9,10 @@ from tests.basic_env import TestDvc
 
 class TestLogger(TestDvc):
     def test_config(self):
-        config1 = {Config.SECTION_CORE: {'LogLevel': 'debug'}}
-        Logger(config1)
+        Logger('debug')
         self.assertEqual(Logger.logger().getEffectiveLevel(), logging.DEBUG)
 
-        config2 = {Config.SECTION_CORE: {'LogLevel': 'error'}}
-        Logger(config2)
+        Logger('error')
         self.assertEqual(Logger.logger().getEffectiveLevel(), logging.ERROR)
 
     def test_set_level(self):