From 505b69fd8b0c2150370a0c1740e4188514e9b271 Mon Sep 17 00:00:00 2001 From: karajan1001 Date: Sun, 3 May 2020 18:00:12 +0800 Subject: [PATCH 1/5] fixed #3599 1. add new command `dvc remote rename `. 2. add tests for this new command. --- dvc/command/remote.py | 39 +++++++++++++++++++++ tests/func/test_remote.py | 71 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+) diff --git a/dvc/command/remote.py b/dvc/command/remote.py index c5f46a1a08..4afc66776a 100644 --- a/dvc/command/remote.py +++ b/dvc/command/remote.py @@ -108,6 +108,34 @@ def run(self): return 0 +class CmdRemoteRename(CmdRemote): + def run(self): + conf = self.config.load_one(self.args.level) + self._check_exists(conf) + + all_config = self.config.load_config_to_level("all") + if self.args.new in all_config.get("remote", {}): + raise ConfigError( + "Rename failed.Remote name {} already exists.".format( + {self.args.new} + ) + ) + + with self.config.edit(self.args.level) as conf: + conf["remote"][self.args.new] = conf["remote"][self.args.name] + del conf["remote"][self.args.name] + + for level in reversed(self.config.LEVELS): + with self.config.edit(level) as conf: + if conf["core"].get("remote") == self.args.name: + conf["core"]["remote"] = self.args.new + + if level == self.args.level: + break + + return 0 + + def add_parser(subparsers, parent_parser): from dvc.command.config import parent_config_parser @@ -224,3 +252,14 @@ def add_parser(subparsers, parent_parser): "name", help="Name of the remote to remove." ) remote_remove_parser.set_defaults(func=CmdRemoteRemove) + REMOTE_RENAME_HELP = "Rename a data remote" + remote_rename_parser = remote_subparsers.add_parser( + "rename", + parents=[parent_config_parser, parent_parser], + description=append_doc_link(REMOTE_RENAME_HELP, "remote/rename"), + help=REMOTE_RENAME_HELP, + formatter_class=argparse.RawDescriptionHelpFormatter, + ) + remote_rename_parser.add_argument("name", help="Remote to be renamed") + remote_rename_parser.add_argument("new", help="New name of the remote") + remote_rename_parser.set_defaults(func=CmdRemoteRename) diff --git a/tests/func/test_remote.py b/tests/func/test_remote.py index 8e349efb98..f447cb6942 100644 --- a/tests/func/test_remote.py +++ b/tests/func/test_remote.py @@ -285,3 +285,74 @@ def test_remote_modify_default(dvc): assert repo_config["core"]["remote"] == remote_repo assert local_config["core"]["remote"] == remote_local + + +def test_remote_rename(dvc): + remote_name = "drive" + remote_url = "gdrive://test/test" + new_name = "new" + other_name = "other" + # prepare + assert main(["remote", "add", remote_name, remote_url]) == 0 + config = dvc.config.load_one("repo") + assert config["remote"][remote_name]["url"] == remote_url + assert new_name not in config.get("remote", {}) + + # rename failed + assert main(["remote", "rename", remote_name]) == 254 + assert main(["remote", "rename", new_name, other_name]) == 251 + config = dvc.config.load_one("repo") + assert config["remote"][remote_name]["url"] == remote_url + assert new_name not in config.get("remote", {}) + + # rename success + assert main(["remote", "rename", remote_name, new_name]) == 0 + config = dvc.config.load_one("repo") + assert remote_name not in config.get("remote", {}) + assert config["remote"][new_name]["url"] == remote_url + + +def test_remote_duplicated(dvc): + remote_name = "drive" + remote_url = "gdrive://test/test" + used_name = "overlap" + another_url = "gdrive://test/test1" + # prepare + assert main(["remote", "add", remote_name, remote_url]) == 0 + assert main(["remote", "add", "--local", used_name, another_url]) == 0 + config = dvc.config.load_one("repo") + assert config["remote"][remote_name]["url"] == remote_url + local_config = dvc.config.load_one("local") + assert local_config["remote"][used_name]["url"] == another_url + + # rename duplicated + assert main(["remote", "rename", remote_name, used_name]) == 251 + config = dvc.config.load_one("repo") + assert config["remote"][remote_name]["url"] == remote_url + local_config = dvc.config.load_one("local") + assert local_config["remote"][used_name]["url"] == another_url + + +def test_remote_default(dvc): + remote_name = "drive" + remote_url = "gdrive://test/test" + new_name = "new" + # prepare + assert main(["remote", "add", "-d", remote_name, remote_url]) == 0 + assert main(["remote", "default", "--local", remote_name]) == 0 + config = dvc.config.load_one("repo") + assert config["core"]["remote"] == remote_name + assert config["remote"][remote_name]["url"] == remote_url + assert new_name not in config.get("remote", {}) + local_config = dvc.config.load_one("local") + assert local_config["core"]["remote"] == remote_name + + # rename success + assert main(["remote", "rename", remote_name, new_name]) == 0 + config = dvc.config.load_one("repo") + assert remote_name not in config.get("remote", {}) + assert config["core"]["remote"] == new_name + assert config["remote"][new_name]["url"] == remote_url + assert remote_name not in config.get("remote", {}) + local_config = dvc.config.load_one("local") + assert local_config["core"]["remote"] == new_name From 200a2f34cf2b6e2ecbec745e2c4f18100e878b86 Mon Sep 17 00:00:00 2001 From: karajan1001 Date: Tue, 19 May 2020 22:44:38 +0800 Subject: [PATCH 2/5] Update dvc/command/remote.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Peter Rowlands (변기호) --- dvc/command/remote.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dvc/command/remote.py b/dvc/command/remote.py index 4afc66776a..e5a68aec5d 100644 --- a/dvc/command/remote.py +++ b/dvc/command/remote.py @@ -116,7 +116,7 @@ def run(self): all_config = self.config.load_config_to_level("all") if self.args.new in all_config.get("remote", {}): raise ConfigError( - "Rename failed.Remote name {} already exists.".format( + "Rename failed. Remote name '{}' already exists.".format( {self.args.new} ) ) From 50abb10ad64d116cc8cd1e5ebfe518b9073e9e5e Mon Sep 17 00:00:00 2001 From: karajan1001 Date: Tue, 19 May 2020 23:13:05 +0800 Subject: [PATCH 3/5] Change about code review --- dvc/command/remote.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/dvc/command/remote.py b/dvc/command/remote.py index 4afc66776a..5864420454 100644 --- a/dvc/command/remote.py +++ b/dvc/command/remote.py @@ -110,10 +110,7 @@ def run(self): class CmdRemoteRename(CmdRemote): def run(self): - conf = self.config.load_one(self.args.level) - self._check_exists(conf) - - all_config = self.config.load_config_to_level("all") + all_config = self.config.load_config_to_level(None) if self.args.new in all_config.get("remote", {}): raise ConfigError( "Rename failed.Remote name {} already exists.".format( @@ -122,17 +119,19 @@ def run(self): ) with self.config.edit(self.args.level) as conf: + self._check_exists(conf) conf["remote"][self.args.new] = conf["remote"][self.args.name] del conf["remote"][self.args.name] + if conf["core"].get("remote") == self.args.name: + conf["core"]["remote"] = self.args.new for level in reversed(self.config.LEVELS): + if level == self.args.level: + break with self.config.edit(level) as conf: if conf["core"].get("remote") == self.args.name: conf["core"]["remote"] = self.args.new - if level == self.args.level: - break - return 0 From bf1a27a442033d0a7849537b001d06a2209a360d Mon Sep 17 00:00:00 2001 From: karajan1001 Date: Tue, 19 May 2020 23:28:08 +0800 Subject: [PATCH 4/5] For deepsource --- dvc/command/remote.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/dvc/command/remote.py b/dvc/command/remote.py index 04c11926f5..b4e9052376 100644 --- a/dvc/command/remote.py +++ b/dvc/command/remote.py @@ -109,6 +109,10 @@ def run(self): class CmdRemoteRename(CmdRemote): + def _rename_default(self, conf): + if conf["core"].get("remote") == self.args.name: + conf["core"]["remote"] = self.args.new + def run(self): all_config = self.config.load_config_to_level(None) if self.args.new in all_config.get("remote", {}): @@ -122,15 +126,13 @@ def run(self): self._check_exists(conf) conf["remote"][self.args.new] = conf["remote"][self.args.name] del conf["remote"][self.args.name] - if conf["core"].get("remote") == self.args.name: - conf["core"]["remote"] = self.args.new + self._rename_default(conf) for level in reversed(self.config.LEVELS): if level == self.args.level: break - with self.config.edit(level) as conf: - if conf["core"].get("remote") == self.args.name: - conf["core"]["remote"] = self.args.new + with self.config.edit(level) as level_conf: + self._rename_default(level_conf) return 0 From 55a4320736cc60eb29b399dff175c11a9e2cb9d5 Mon Sep 17 00:00:00 2001 From: karajan1001 Date: Wed, 20 May 2020 15:14:39 +0800 Subject: [PATCH 5/5] Update dvc/command/remote.py --- dvc/command/remote.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dvc/command/remote.py b/dvc/command/remote.py index b4e9052376..2dfaf80d7f 100644 --- a/dvc/command/remote.py +++ b/dvc/command/remote.py @@ -253,7 +253,7 @@ def add_parser(subparsers, parent_parser): "name", help="Name of the remote to remove." ) remote_remove_parser.set_defaults(func=CmdRemoteRemove) - REMOTE_RENAME_HELP = "Rename a data remote" + REMOTE_RENAME_HELP = "Rename a DVC remote" remote_rename_parser = remote_subparsers.add_parser( "rename", parents=[parent_config_parser, parent_parser],