From 1ad63d1ba6dbb7d375c01a686718aab0a6d8497c Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Sat, 7 Apr 2018 01:21:05 +0300 Subject: [PATCH 1/3] dvc: add `dvc remote modify` command Signed-off-by: Ruslan Kuprieiev --- dvc/cli.py | 24 +++++++++++++++++++++++- dvc/command/remote.py | 7 +++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/dvc/cli.py b/dvc/cli.py index cfbd99356..09f5fd5d1 100644 --- a/dvc/cli.py +++ b/dvc/cli.py @@ -16,7 +16,7 @@ from dvc.command.gc import CmdGC from dvc.command.add import CmdAdd from dvc.command.config import CmdConfig from dvc.command.checkout import CmdCheckout -from dvc.command.remote import CmdRemoteAdd, CmdRemoteRemove +from dvc.command.remote import CmdRemoteAdd, CmdRemoteRemove, CmdRemoteModify from dvc.stage import Stage from dvc import VERSION @@ -275,6 +275,28 @@ def parse_args(argv=None): remote_remove_parser.set_defaults(func=CmdRemoteRemove) + remote_modify_parser = remote_subparsers.add_parser( + 'modify', + parents=[parent_parser], + help='Modify remote') + remote_modify_parser.add_argument( + 'name', + help='Name') + remote_modify_parser.add_argument( + 'option', + help='Option') + remote_modify_parser.add_argument( + 'value', + nargs='?', + help='Value') + remote_modify_parser.add_argument('-u', + '--unset', + default=False, + action='store_true', + help='Unset option') + remote_modify_parser.set_defaults(func=CmdRemoteModify) + + args = parser.parse_args(argv) return args diff --git a/dvc/command/remote.py b/dvc/command/remote.py index a822b36e4..48c0dd853 100644 --- a/dvc/command/remote.py +++ b/dvc/command/remote.py @@ -13,3 +13,10 @@ class CmdRemoteRemove(CmdConfig): def run(self): section = Config.SECTION_REMOTE_FMT.format(self.args.name) return self.unset(section, Config.SECTION_REMOTE_URL) + + +class CmdRemoteModify(CmdConfig): + def run(self): + section = Config.SECTION_REMOTE_FMT.format(self.args.name) + self.args.name = '{}.{}'.format(section, self.args.option) + return super(CmdRemoteModify, self).run() From e8b5af6cd97d37ea4b819e7a3d86f62c911205a8 Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Sat, 7 Apr 2018 02:34:00 +0300 Subject: [PATCH 2/3] test: test remotes in CLI Signed-off-by: Ruslan Kuprieiev --- tests/test_data_cloud.py | 199 ++++++++++++++++++++++++++++----------- 1 file changed, 145 insertions(+), 54 deletions(-) diff --git a/tests/test_data_cloud.py b/tests/test_data_cloud.py index 4df0cafe3..d6a6aa8d0 100644 --- a/tests/test_data_cloud.py +++ b/tests/test_data_cloud.py @@ -2,6 +2,7 @@ import os import stat import uuid import shutil +import tempfile from dvc.main import main from dvc.config import Config, ConfigError @@ -17,6 +18,58 @@ TEST_SECTION = 'remote "{}"'.format(TEST_REMOTE) TEST_CONFIG = {Config.SECTION_CORE: {Config.SECTION_CORE_REMOTE: TEST_REMOTE}, TEST_SECTION: {Config.SECTION_REMOTE_URL: ''}} +TEST_AWS_REPO_BUCKET = 'dvc-test' +TEST_AWS_REPO_REGION = 'us-east-2' + +TEST_GCP_REPO_BUCKET = 'dvc-test' + + +def should_test_aws(): + dvc_test_aws = os.getenv("DVC_TEST_AWS") + if dvc_test_aws == "true": + return True + elif dvc_test_aws == "false": + return False + + if os.getenv("AWS_ACCESS_KEY_ID") and os.getenv("AWS_SECRET_ACCESS_KEY"): + return True + + return False + + +def should_test_gcp(): + if os.getenv("DVC_TEST_GCP") == "true": + return True + + if os.getenv("GOOGLE_APPLICATION_CREDENTIALS") and os.getenv("GCP_CREDS"): + return True + + return False + + +def get_local_storagepath(): + return tempfile.mkdtemp() + + +def get_local_url(): + return get_local_storagepath() + + +def get_aws_storagepath(): + return TEST_AWS_REPO_BUCKET + '/' + str(uuid.uuid4()) + + +def get_aws_url(): + return 's3://' + get_aws_storagepath() + + +def get_gcp_storagepath(): + return TEST_GCP_REPO_BUCKET + '/' + str(uuid.uuid4()) + + +def get_gcp_url(): + return 'gs://' + get_gcp_storagepath() + class TestDataCloud(TestDvc): def _test_cloud(self, config, cl): @@ -106,59 +159,33 @@ class TestDataCloudBase(TestDvc): class TestDataCloudAWS(TestDataCloudBase): - TEST_REPO_BUCKET = 'dvc-test' - TEST_REPO_REGION = 'us-east-2' - - def _should_test(self): - dvc_test_aws = os.getenv("DVC_TEST_AWS") - if dvc_test_aws == "true": - return True - elif dvc_test_aws == "false": - return False - - if os.getenv("AWS_ACCESS_KEY_ID") and os.getenv("AWS_SECRET_ACCESS_KEY"): - return True - - return False - def _setup_cloud(self): - if not self._should_test(): + if not should_test_aws(): return - repo = 's3://' + self.TEST_REPO_BUCKET + '/' + str(uuid.uuid4()) + repo = get_aws_url() # Setup cloud config = TEST_CONFIG config[TEST_SECTION][Config.SECTION_REMOTE_URL] = repo - config[TEST_SECTION][Config.SECTION_AWS_REGION] = self.TEST_REPO_REGION + config[TEST_SECTION][Config.SECTION_AWS_REGION] = TEST_AWS_REPO_REGION cloud_settings = CloudSettings(self.dvc.cache, None, config[TEST_SECTION]) self.cloud = DataCloudAWS(cloud_settings) def test(self): - if self._should_test(): + if should_test_aws(): self._test_cloud() class TestDataCloudGCP(TestDataCloudBase): - TEST_REPO_GCP_BUCKET = 'dvc-test' - - def _should_test(self): - if os.getenv("DVC_TEST_GCP") == "true": - return True - - if os.getenv("GOOGLE_APPLICATION_CREDENTIALS") and os.getenv("GCP_CREDS"): - return True - - return False - def _setup_cloud(self): - if not self._should_test(): + if not should_test_gcp(): return if os.getenv("GOOGLE_APPLICATION_CREDENTIALS") and os.getenv("GCP_CREDS"): shutil.copyfile(self.GCP_CREDS_FILE, os.getenv("GOOGLE_APPLICATION_CREDENTIALS")) - repo = 'gs://' + self.TEST_REPO_GCP_BUCKET + '/' + str(uuid.uuid4()) + repo = get_gcp_url() # Setup cloud config = TEST_CONFIG @@ -167,17 +194,13 @@ class TestDataCloudGCP(TestDataCloudBase): self.cloud = DataCloudGCP(cloud_settings) def test(self): - if self._should_test(): + if should_test_gcp(): self._test_cloud() class TestDataCloudLOCAL(TestDataCloudBase): - def _should_test(self): - return True - def _setup_cloud(self): - self.dname = 'cloud' - os.mkdir(self.dname) + self.dname = get_local_url() config = TEST_CONFIG config[TEST_SECTION][Config.SECTION_REMOTE_URL] = self.dname @@ -185,22 +208,20 @@ class TestDataCloudLOCAL(TestDataCloudBase): self.cloud = DataCloudLOCAL(cloud_settings) def test(self): - if self._should_test(): - self._test_cloud() - self.assertTrue(os.path.isdir(self.dname)) + self._test_cloud() + self.assertTrue(os.path.isdir(self.dname)) -class TestDataCloudLocalCli(TestDvc): +class TestDataCloudCLIBase(TestDvc): def main(self, args): ret = main(args) self.assertEqual(ret, 0) - def test(self): - dname = 'cloud' - os.mkdir(dname) - - self.main(['config', 'core.cloud', 'local']) - self.main(['config', 'local.storagepath', dname]) + def _test_cloud(self, remote=None): + if remote: + remote_arg = ['-r', remote] + else: + remote_arg = [] stage = self.dvc.add(self.FOO) cache = stage.outs[0].cache @@ -209,27 +230,97 @@ class TestDataCloudLocalCli(TestDvc): cache_dir = stage_dir.outs[0].cache #FIXME check status output - self.main(['status']) + self.main(['status', '-c'] + remote_arg) - self.main(['push']) + self.main(['push'] + remote_arg) self.assertTrue(os.path.exists(cache)) self.assertTrue(os.path.isfile(cache)) self.assertTrue(os.path.isfile(cache_dir)) - self.main(['status']) + self.main(['status', '-c'] + remote_arg) shutil.rmtree(self.dvc.cache.cache_dir) - self.main(['status']) + self.main(['status', '-c'] + remote_arg) - self.main(['pull']) + self.main(['pull'] + remote_arg) 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.isfile(cache_dir)) - self.main(['status']) + self.main(['status', '-c'] + remote_arg) + + def should_test(self): + return True + + def _test(self): + pass + + def _test_compat(self): + pass + + def test(self): + if self.should_test(): + self._test() + self._test_compat() + + +class TestDataCloudLOCALCLI(TestDataCloudCLIBase): + def _test_compat(self): + storagepath = get_local_storagepath() + self.main(['config', 'core.cloud', 'local']) + self.main(['config', 'local.storagepath', storagepath]) + + self._test_cloud() + + def _test(self): + url = get_local_url() + + self.main(['remote', 'add', TEST_REMOTE, url]) + + self._test_cloud(TEST_REMOTE) + + +class TestDataCloudAWSCLI(TestDataCloudCLIBase): + def should_test(self): + return should_test_aws() + + def _test_compat(self): + storagepath = get_aws_storagepath() + self.main(['config', 'core.cloud', 'aws']) + self.main(['config', 'aws.storagepath', storagepath]) + self.main(['config', 'aws.region', TEST_AWS_REPO_REGION]) + + self._test_cloud() + + def _test(self): + url = get_aws_url() + + self.main(['remote', 'add', TEST_REMOTE, url]) + self.main(['remote', 'modify', TEST_REMOTE, Config.SECTION_AWS_REGION, TEST_AWS_REPO_REGION]) + + self._test_cloud(TEST_REMOTE) + + +class TestDataCloudGCPCLI(TestDataCloudCLIBase): + def should_test(self): + return should_test_gcp() + + def _test_compat(self): + storagepath = get_gcp_storagepath() + self.main(['config', 'core.cloud', 'gcp']) + self.main(['config', 'gcp.storagepath', storagepath]) + + self._test_cloud() + + def _test(self): + url = get_gcp_url() + + self.main(['remote', 'add', TEST_REMOTE, url]) + + self._test_cloud(TEST_REMOTE) class TestDataCloudErrorCLI(TestDvc): From 8d8af0518e8df80e14a10ef03654d1b9637c2c85 Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Sat, 7 Apr 2018 03:26:05 +0300 Subject: [PATCH 3/3] config: schema: support windows paths Signed-off-by: Ruslan Kuprieiev --- dvc/cloud/base.py | 2 +- dvc/cloud/data_cloud.py | 14 ++++++-------- dvc/config.py | 2 +- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/dvc/cloud/base.py b/dvc/cloud/base.py index 20f352e98..8f2b3d02d 100644 --- a/dvc/cloud/base.py +++ b/dvc/cloud/base.py @@ -53,7 +53,7 @@ class DataCloudBase(object): if path: Logger.warn('Using obsoleted config format. Consider updating.') else: - path = re.match(Config.SECTION_REMOTE_URL_REGEX, url).group(2) + path = re.match(Config.SECTION_REMOTE_URL_REGEX, url).group('storagepath') return path diff --git a/dvc/cloud/data_cloud.py b/dvc/cloud/data_cloud.py index ded861a63..5770f75a8 100644 --- a/dvc/cloud/data_cloud.py +++ b/dvc/cloud/data_cloud.py @@ -1,10 +1,4 @@ import re - -try: - from urlparse import urlparse -except ImportError: - from urllib.parse import urlparse - from multiprocessing.pool import ThreadPool from dvc.cloud.instance_manager import CloudSettings @@ -32,7 +26,7 @@ class DataCloud(object): SCHEME_MAP = { 's3' : 'aws', 'gs' : 'gcp', - '' : 'local', + None : 'local', } def __init__(self, cache, config): @@ -58,7 +52,11 @@ class DataCloud(object): raise ConfigError("Can't find remote section '{}' in config".format(section)) url = cloud_config[Config.SECTION_REMOTE_URL] - scheme = urlparse(url).scheme + 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) if not cloud_type: raise ConfigError("Unsupported scheme '{}' in '{}'".format(scheme, url)) diff --git a/dvc/config.py b/dvc/config.py index 8b9b222bb..99b8bdf96 100644 --- a/dvc/config.py +++ b/dvc/config.py @@ -69,7 +69,7 @@ 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*(s3://|gs://|/)?(.*)\s*$' + SECTION_REMOTE_URL_REGEX = r'^\s*(((?Ps3|gs|..+):/*)|/*|.\\*)?(?P.*)\s*$' SECTION_REMOTE_SCHEMA = { SECTION_REMOTE_URL: schema.Regex(SECTION_REMOTE_URL_REGEX), schema.Optional(SECTION_AWS_REGION): str,