From 5a8aef28f52e461fb1a8c127da55e1fcc2e859ec Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Tue, 3 Apr 2018 02:07:49 +0300 Subject: [PATCH] config: empty config by default + format verification Signed-off-by: Ruslan Kuprieiev --- dvc/cloud/aws.py | 5 ++- dvc/cloud/base.py | 4 +- dvc/cloud/credentials_aws.py | 5 ++- dvc/cloud/data_cloud.py | 53 +++++++++--------------- dvc/cloud/gcp.py | 6 +-- dvc/config.py | 71 ++++++++++++++++++++++++-------- tests/test_config.py | 80 +++++++++++++++--------------------- tests/test_data_cloud.py | 22 +++++----- 8 files changed, 127 insertions(+), 119 deletions(-) diff --git a/dvc/cloud/aws.py b/dvc/cloud/aws.py index 90cb46b79..bb1c517b1 100644 --- a/dvc/cloud/aws.py +++ b/dvc/cloud/aws.py @@ -12,6 +12,7 @@ except ImportError: sys.modules['httplib'] = httplib from boto.s3.resumable_download_handler import ResumableDownloadHandler +from dvc.config import Config from dvc.logger import Logger from dvc.progress import progress from dvc.cloud.credentials_aws import AWSCredentials @@ -57,7 +58,7 @@ class DataCloudAWS(DataCloudBase): See notes http://docs.aws.amazon.com/general/latest/gr/rande.html#s3_region """ - region = self._cloud_settings.cloud_config.get('Region', None) + region = self._cloud_settings.cloud_config.get(Config.SECTION_AWS_REGION, None) if region is None or region == '': return 's3.amazonaws.com' if region == 'us-east-1': @@ -69,7 +70,7 @@ class DataCloudAWS(DataCloudBase): Try obtaining path to aws credentials from config file. """ paths = [] - credpath = self._cloud_settings.cloud_config.get('CredentialPath', None) + credpath = self._cloud_settings.cloud_config.get(Config.SECTION_AWS_CREDENTIALPATH, None) if credpath is not None and len(credpath) > 0: credpath = os.path.expanduser(credpath) if os.path.isfile(credpath): diff --git a/dvc/cloud/base.py b/dvc/cloud/base.py index 3a23a23e6..9199afeef 100644 --- a/dvc/cloud/base.py +++ b/dvc/cloud/base.py @@ -4,7 +4,7 @@ from checksumdir import dirhash from dvc.logger import Logger from dvc.exceptions import DvcException -from dvc.config import ConfigError +from dvc.config import Config, ConfigError from dvc.stage import Output @@ -45,7 +45,7 @@ class DataCloudBase(object): if self._cloud_settings.global_storage_path: return self._cloud_settings.global_storage_path - path = self._cloud_settings.cloud_config.get('StoragePath', None) + path = self._cloud_settings.cloud_config.get(Config.SECTION_CORE_STORAGEPATH, None) if path is None: raise ConfigError('invalid StoragePath: not set for Data or cloud specific') diff --git a/dvc/cloud/credentials_aws.py b/dvc/cloud/credentials_aws.py index e28083ba6..1885a0ea1 100644 --- a/dvc/cloud/credentials_aws.py +++ b/dvc/cloud/credentials_aws.py @@ -1,14 +1,15 @@ import os import configparser +from dvc.config import Config from dvc.utils import cached_property from dvc.logger import Logger class AWSCredentials(object): def __init__(self, cloud_config): - self._conf_credpath = cloud_config.get('CredentialPath', None) - self._conf_credsect = cloud_config.get('Profile', 'default') + self._conf_credpath = cloud_config.get(Config.SECTION_AWS_CREDENTIALPATH, None) + self._conf_credsect = cloud_config.get(Config.SECTION_AWS_PROFILE, 'default') @property def access_key_id(self): diff --git a/dvc/cloud/data_cloud.py b/dvc/cloud/data_cloud.py index 3080da018..d0415f9be 100644 --- a/dvc/cloud/data_cloud.py +++ b/dvc/cloud/data_cloud.py @@ -10,27 +10,34 @@ from dvc.config import Config from dvc.cloud.aws import DataCloudAWS from dvc.cloud.gcp import DataCloudGCP from dvc.cloud.local import DataCloudLOCAL +from dvc.cloud.base import DataCloudBase class DataCloud(object): """ Generic class to do initial config parsing and redirect to proper DataCloud methods """ CLOUD_MAP = { - 'AWS' : DataCloudAWS, - 'GCP' : DataCloudGCP, - 'LOCAL' : DataCloudLOCAL, + 'aws' : DataCloudAWS, + 'gcp' : DataCloudGCP, + 'local' : DataCloudLOCAL, } SCHEME_MAP = { - 's3' : 'AWS', - 'gs' : 'GCP', - '' : 'LOCAL', + 's3' : 'aws', + 'gs' : 'gcp', + '' : 'local', } def __init__(self, cache, config): self._config = config - cloud_type = self._config[Config.SECTION_CORE].get('Cloud', '').strip().upper() + cloud_type = self._config[Config.SECTION_CORE].get(Config.SECTION_CORE_CLOUD, '').strip().lower() + + if cloud_type == '': + self.typ = None + self._cloud = None + return + if cloud_type not in self.CLOUD_MAP.keys(): raise ConfigError('Wrong cloud type %s specified' % cloud_type) @@ -43,8 +50,7 @@ class DataCloud(object): self.typ = cloud_type self._cloud = self.CLOUD_MAP[cloud_type](cloud_settings) - - self.sanity_check() + self._cloud.sanity_check() @staticmethod def get_cloud_settings(cache, config, cloud_type): @@ -55,34 +61,10 @@ class DataCloud(object): cloud_config = None else: cloud_config = config[cloud_type] - global_storage_path = config[Config.SECTION_CORE].get('StoragePath', None) + global_storage_path = config[Config.SECTION_CORE].get(Config.SECTION_CORE_STORAGEPATH, None) cloud_settings = CloudSettings(cache, global_storage_path, cloud_config) return cloud_settings - 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 - """ - key = 'Cloud' - core = self._config[Config.SECTION_CORE] - if key.lower() not in [k.lower() for k in core.keys()] or len(core[key]) < 1: - raise ConfigError('Please set {} in section {} in config file'.format(key, Config.SECTION_CORE)) - - # now that a cloud is chosen, can check StoragePath - storage_path = self._cloud.storage_path - if storage_path is None or len(storage_path) == 0: - raise ConfigError('Please set StoragePath = bucket/{optional path} ' - 'in config file in a cloud specific section') - - self._cloud.sanity_check() - def _collect(self, targets, jobs, local): collected = set() pool = ThreadPool(processes=jobs) @@ -98,6 +80,9 @@ class DataCloud(object): """ Process targets as data items in parallel. """ + if not self._cloud: + return + self._cloud.connect() collected = set() diff --git a/dvc/cloud/gcp.py b/dvc/cloud/gcp.py index 1a8ae4917..0969f1355 100644 --- a/dvc/cloud/gcp.py +++ b/dvc/cloud/gcp.py @@ -4,7 +4,7 @@ import os from google.cloud import storage as gc from dvc.logger import Logger -from dvc.config import ConfigError +from dvc.config import Config, ConfigError from dvc.cloud.base import DataCloudError, DataCloudBase from dvc.utils import file_md5 from dvc.progress import progress @@ -17,13 +17,13 @@ class DataCloudGCP(DataCloudBase): """ Get project name from config. """ - return self._cloud_settings.cloud_config.get('ProjectName', None) + return self._cloud_settings.cloud_config.get(Config.SECTION_GCP_PROJECTNAME, None) def sanity_check(self): project = self.gc_project_name if project is None or len(project) < 1: raise ConfigError('can\'t read google cloud project name. ' - 'Please set ProjectName in section GC.') + 'Please set \'{}\' in section GCP.'.format(Config.SECTION_GCP_PROJECTNAME)) def connect(self): client = gc.Client(project=self.gc_project_name) diff --git a/dvc/config.py b/dvc/config.py index 4f2c3dc70..3b713e1ba 100644 --- a/dvc/config.py +++ b/dvc/config.py @@ -2,6 +2,7 @@ DVC config objects. """ import os +import schema import configobj from dvc.exceptions import DvcException @@ -15,26 +16,51 @@ class ConfigError(DvcException): class Config(object): CONFIG = 'config' - SECTION_CORE = 'Core' - CONFIG_TEMPLATE = ''' -[Core] -# Supported clouds: AWS, GCP -Cloud = AWS -# Log levels: Debug, Info, Warning and Error -LogLevel = Info + SECTION_CORE = 'core' + SECTION_CORE_LOGLEVEL = 'loglevel' + SECTION_CORE_LOGLEVEL_SCHEMA = schema.And(schema.Use(str.lower), lambda l: l in ('info', 'debug', 'warning', 'error')) + SECTION_CORE_CLOUD = 'cloud' + SECTION_CORE_CLOUD_SCHEMA = schema.And(schema.Use(str.lower), lambda c: c in ('aws', 'gcp', 'local', '')) + SECTION_CORE_STORAGEPATH = 'storagepath' # backward compatibility + SECTION_CORE_SCHEMA = { + schema.Optional(SECTION_CORE_LOGLEVEL, default='info'): SECTION_CORE_LOGLEVEL_SCHEMA, + schema.Optional(SECTION_CORE_CLOUD, default=''): SECTION_CORE_CLOUD_SCHEMA, + schema.Optional(SECTION_CORE_STORAGEPATH, default=''): str, + } -[AWS] -CredentialPath = ~/.aws/credentials -Profile = default + SECTION_AWS = 'aws' + SECTION_AWS_STORAGEPATH = 'storagepath' + SECTION_AWS_CREDENTIALPATH = 'credentialpath' + SECTION_AWS_REGION = 'region' + SECTION_AWS_PROFILE = 'profile' + SECTION_AWS_SCHEMA = { + SECTION_AWS_STORAGEPATH: str, + schema.Optional(SECTION_AWS_REGION): str, + schema.Optional(SECTION_AWS_PROFILE, default='default'): str, + schema.Optional(SECTION_AWS_CREDENTIALPATH, default = ''): str, + } -StoragePath = dvc/tutorial + SECTION_GCP = 'gcp' + SECTION_GCP_STORAGEPATH = SECTION_AWS_STORAGEPATH + SECTION_GCP_PROJECTNAME = 'projectname' + SECTION_GCP_SCHEMA = { + SECTION_GCP_STORAGEPATH: str, + SECTION_GCP_PROJECTNAME: str, + } -[GCP] -StoragePath = -ProjectName = + SECTION_LOCAL = 'local' + SECTION_LOCAL_STORAGEPATH = SECTION_AWS_STORAGEPATH + SECTION_LOCAL_SCHEMA = { + SECTION_LOCAL_STORAGEPATH: str, + } -''' + SCHEMA = { + schema.Optional(SECTION_CORE, default={}): SECTION_CORE_SCHEMA, + schema.Optional(SECTION_AWS, default={}): SECTION_AWS_SCHEMA, + schema.Optional(SECTION_GCP, default={}): SECTION_GCP_SCHEMA, + schema.Optional(SECTION_LOCAL, default={}): SECTION_LOCAL_SCHEMA, + } def __init__(self, dvc_dir): self.dvc_dir = os.path.abspath(os.path.realpath(dvc_dir)) @@ -42,14 +68,23 @@ ProjectName = try: self._config = configobj.ConfigObj(self.config_file, write_empty_values=True) + self._config = self._lower(self._config) + self._config = schema.Schema(self.SCHEMA).validate(self._config) except Exception as ex: raise ConfigError(ex.message) - if self.SECTION_CORE not in self._config.keys(): - raise ConfigError(u'section \'{}\' was not found'.format(self.SECTION_CORE)) + @staticmethod + def _lower(config): + new_config = {} + for s_key, s_value in config.items(): + new_s = {} + for key, value in s_value.items(): + new_s[key.lower()] = value + new_config[s_key.lower()] = new_s + return new_config @staticmethod def init(dvc_dir): config_file = os.path.join(dvc_dir, Config.CONFIG) - open(config_file, 'w').write(Config.CONFIG_TEMPLATE) + open(config_file, 'w+').close() return Config(dvc_dir) diff --git a/tests/test_config.py b/tests/test_config.py index 1fb84aed4..76d03887c 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1,5 +1,4 @@ import os -import configparser import configobj from unittest import TestCase import mock @@ -26,49 +25,42 @@ class TestConfigTest(TestCase): pass def _conf(self, s): - c = configparser.SafeConfigParser() - c.readfp(s) + c = configobj.ConfigObj(s) return c def storage_path_hierarchy_test(self): """ StoragePath should be read first from global and then from cloud section """ - c = ("[Core]", - "LogLevel =", - "DataDir = ", - "CacheDir = ", - "StateDir = ", - "Cloud = aws", - "StoragePath = globalsb/global_storage_path", - "[AWS]", - "StoragePath = awssb/aws_storage_path", - "CredentialPath =", - "[GCP]", - "StoragePath = googlesb/google_storage_path" + c = ("[core]", + "loglevel =", + "cloud = aws", + "storagepath = globalsb/global_storage_path", + "[aws]", + "storagepath = awssb/aws_storage_path", + "credentialpath =", + "[gcp]", + "storagepath = googlesb/google_storage_path" ) s = StringIO('\n'.join(c)) conf = self._conf(s) cloud = DataCloud(None, conf) - self.assertEqual(cloud.typ, 'AWS') + self.assertEqual(cloud.typ, 'aws') self.assertEqual(cloud._cloud.storage_bucket, 'globalsb') self.assertEqual(cloud._cloud.storage_prefix, 'global_storage_path') - c = ("[Core]", - "LogLevel =", - "DataDir = ", - "CacheDir = ", - "StateDir = ", - "Cloud = Aws", - "[AWS]", - "StoragePath = awssb/aws_storage_path", - "CredentialPath =", - "[GCP]", - "StoragePath = googlesb/google_storage_path" + c = ("[core]", + "loglevel =", + "cloud = Aws", + "[aws]", + "storagepath = awssb/aws_storage_path", + "credentialpath =", + "[gcp]", + "storagepath = googlesb/google_storage_path" ) s = StringIO('\n'.join(c)) conf = self._conf(s) cloud = DataCloud(None, conf) - self.assertEqual(cloud.typ, 'AWS') + self.assertEqual(cloud.typ, 'aws') self.assertEqual(cloud._cloud.storage_bucket, 'awssb') self.assertEqual(cloud._cloud.storage_prefix, 'aws_storage_path') @@ -90,15 +82,12 @@ class TestConfigTest(TestCase): """ in absence of [AWS] -> CredentialPath, aws creds should be read from ~/.aws/credentials """ default_path = os.path.expanduser('~/.aws/credentials') - c = ("[Core]", - "LogLevel =", - "DataDir = ", - "CacheDir = ", - "StateDir = ", - "Cloud = aws", - "[AWS]", - "StoragePath = awssb/aws_storage_path", - "CredentialPath =", + c = ("[core]", + "loglevel =", + "cloud = aws", + "[aws]", + "storagepath = awssb/aws_storage_path", + "credentialpath =", ) s = StringIO('\n'.join(c)) @@ -121,15 +110,12 @@ class TestConfigTest(TestCase): def aws_credentials_specified_tes_t_DISABLED(self, isfile_function): """ in presence of [AWS] -> CredentialPath, use those credentials """ - c = ("[Core]", - "LogLevel =", - "DataDir = ", - "CacheDir = ", - "StateDir = ", - "Cloud = aws", - "[AWS]", - "StoragePath = awssb/aws_storage_path", - "CredentialPath = some_credential_path", + c = ("[core]", + "loglevel =", + "cloud = aws", + "[aws]", + "storagePath = awssb/aws_storage_path", + "credentialpath = some_credential_path", ) s = StringIO('\n'.join(c)) @@ -160,7 +146,7 @@ class TestConfigCLI(TestDvc): return True def test(self): - section = 'SetSection' + section = 'setsection' field = 'setfield' section_field = '{}.{}'.format(section, field) value = 'setvalue' diff --git a/tests/test_data_cloud.py b/tests/test_data_cloud.py index b1e907e16..561cec72c 100644 --- a/tests/test_data_cloud.py +++ b/tests/test_data_cloud.py @@ -15,14 +15,14 @@ from tests.basic_env import TestDvc class TestDataCloud(TestDvc): def test(self): for k,v in DataCloud.CLOUD_MAP.items(): - config = {Config.SECTION_CORE: {'Cloud': k}, - k: {'StoragePath': 'a/b', - 'ProjectName': 'name'}} + config = {Config.SECTION_CORE: {'cloud': k}, + k: {'storagepath': 'a/b', + 'projectname': 'name'}} cloud = DataCloud(self.dvc.cache, config) self.assertIsInstance(cloud._cloud, v) with self.assertRaises(ConfigError) as cx: - config = {Config.SECTION_CORE: {'Cloud': 'not_supported_type'}} + config = {Config.SECTION_CORE: {'cloud': 'not_supported_type'}} DataCloud(self.dvc.cache, config) @@ -117,8 +117,8 @@ class TestDataCloudAWS(TestDataCloudBase): repo = self.TEST_REPO_BUCKET + '/' + str(uuid.uuid4()) # Setup cloud - config = {'StoragePath': repo, - 'Region': self.TEST_REPO_REGION} + config = {'storagepath': repo, + 'region': self.TEST_REPO_REGION} cloud_settings = CloudSettings(self.dvc.cache, None, config) self.cloud = DataCloudAWS(cloud_settings) @@ -150,8 +150,8 @@ class TestDataCloudGCP(TestDataCloudBase): repo = self.TEST_REPO_GCP_BUCKET + '/' + str(uuid.uuid4()) # Setup cloud - config = {'StoragePath': repo, - 'Region': self.TEST_REPO_GCP_PROJECT} + config = {'storagepath': repo, + 'region': self.TEST_REPO_GCP_PROJECT} cloud_settings = CloudSettings(self.dvc.cache, None, config) self.cloud = DataCloudGCP(cloud_settings) @@ -167,7 +167,7 @@ class TestDataCloudLOCAL(TestDataCloudBase): def _setup_cloud(self): self.dname = 'cloud' os.mkdir(self.dname) - config = {'StoragePath': self.dname} + config = {'storagepath': self.dname} cloud_settings = CloudSettings(self.dvc.cache, None, config) self.cloud = DataCloudLOCAL(cloud_settings) @@ -186,8 +186,8 @@ class TestDataCloudLocalCli(TestDvc): dname = 'cloud' os.mkdir(dname) - self.main(['config', 'Core.cloud', 'LOCAL']) - self.main(['config', 'LOCAL.StoragePath', dname]) + self.main(['config', 'core.cloud', 'local']) + self.main(['config', 'local.storagepath', dname]) stage = self.dvc.add(self.FOO) cache = stage.outs[0].cache