From f74311414d76355042b1fa28b51040baeb3819fe Mon Sep 17 00:00:00 2001 From: Dominik Gresch Date: Mon, 9 Dec 2019 18:12:26 +0100 Subject: [PATCH 01/14] Add 'verdi node repo cp' command. The command takes a node pk / uuid, zero or more file names, and an ouput directory as arguments. By default, it will create a sub-directory with the node uuid in the output directory, and copy the files with the given names from the repository to that directory. If no file names are given, all objects in the repository are copied. The command has two optional flags: --no-uuid: copies the files directly into the output directory, without uuid prefix --force: Copy files even if they already exist at the target destination --- .../tests/cmdline/commands/test_node.py | 114 ++++++++++++++++++ aiida/cmdline/commands/cmd_node.py | 36 ++++++ 2 files changed, 150 insertions(+) diff --git a/aiida/backends/tests/cmdline/commands/test_node.py b/aiida/backends/tests/cmdline/commands/test_node.py index ac596008f7..629f2673ad 100644 --- a/aiida/backends/tests/cmdline/commands/test_node.py +++ b/aiida/backends/tests/cmdline/commands/test_node.py @@ -10,6 +10,7 @@ """Tests for verdi node""" import os +import io import errno import tempfile @@ -52,6 +53,17 @@ def setUpClass(cls, *args, **kwargs): cls.node = node + # Set up a FolderData for the node repo cp tests. + folder_node = orm.FolderData() + cls.content_file1 = 'nobody expects' + cls.content_file2 = 'the minister of silly walks' + cls.filename_1 = 'test.txt' + cls.filename_2 = 'else.txt' + folder_node.put_object_from_filelike(io.StringIO(cls.content_file1), key=cls.filename_1) + folder_node.put_object_from_filelike(io.StringIO(cls.content_file2), key=cls.filename_2) + folder_node.store() + cls.folder_node = folder_node + def setUp(self): self.cli_runner = CliRunner() @@ -171,6 +183,108 @@ def test_node_extras(self): result = self.cli_runner.invoke(cmd_node.extras, options) self.assertIsNone(result.exception, result.output) + def test_node_repo_cp(self): + """Test 'verdi node repo cp' command.""" + + with tempfile.TemporaryDirectory() as tmp_dir: + options = [str(self.folder_node.uuid), tmp_dir] + self.cli_runner.invoke(cmd_node.repo_cp, options) + + out_dir = os.path.join(tmp_dir, str(self.folder_node.uuid)) + self.assertEqual(sorted(os.listdir(out_dir)), sorted([self.filename_1, self.filename_2])) + with open(os.path.join(out_dir, self.filename_1), 'r') as file_1: + self.assertEqual(file_1.read(), self.content_file1) + with open(os.path.join(out_dir, self.filename_2), 'r') as file_2: + self.assertEqual(file_2.read(), self.content_file2) + + def test_node_repo_cp_no_uuid(self): + """Test 'verdi node repo cp' command with '--no-uuid' option.""" + + with tempfile.TemporaryDirectory() as tmp_dir: + options = [str(self.folder_node.uuid), '--no-uuid', tmp_dir] + self.cli_runner.invoke(cmd_node.repo_cp, options) + + self.assertEqual(sorted(os.listdir(tmp_dir)), sorted([self.filename_1, self.filename_2])) + with open(os.path.join(tmp_dir, self.filename_1), 'r') as file_1: + self.assertEqual(file_1.read(), self.content_file1) + with open(os.path.join(tmp_dir, self.filename_2), 'r') as file_2: + self.assertEqual(file_2.read(), self.content_file2) + + def test_node_repo_cp_one_file(self): + """Test 'verdi node repo cp' command with explicitly specified file name.""" + + with tempfile.TemporaryDirectory() as tmp_dir: + options = [str(self.folder_node.uuid), self.filename_1, tmp_dir] + self.cli_runner.invoke(cmd_node.repo_cp, options) + + out_dir = os.path.join(tmp_dir, str(self.folder_node.uuid)) + self.assertEqual(os.listdir(out_dir), [self.filename_1]) + with open(os.path.join(out_dir, self.filename_1), 'r') as file_1: + self.assertEqual(file_1.read(), self.content_file1) + + def test_node_repo_cp_two_files_explicit(self): + """Test 'verdi node repo cp' command with two explicitly specified file names.""" + + with tempfile.TemporaryDirectory() as tmp_dir: + options = [str(self.folder_node.uuid), self.filename_2, self.filename_1, tmp_dir] + self.cli_runner.invoke(cmd_node.repo_cp, options) + + out_dir = os.path.join(tmp_dir, str(self.folder_node.uuid)) + self.assertEqual(sorted(os.listdir(out_dir)), sorted([self.filename_1, self.filename_2])) + with open(os.path.join(out_dir, self.filename_1), 'r') as file_1: + self.assertEqual(file_1.read(), self.content_file1) + with open(os.path.join(out_dir, self.filename_2), 'r') as file_2: + self.assertEqual(file_2.read(), self.content_file2) + + def test_node_repo_inexistent(self): + """Test 'verdi node repo cp' command with an inexistent file name.""" + + with tempfile.TemporaryDirectory() as tmp_dir: + options = [str(self.folder_node.uuid), 'inexistent_filename', tmp_dir] + result = self.cli_runner.invoke(cmd_node.repo_cp, options) + + self.assertIn('Warning', result.output) + + out_dir = os.path.join(tmp_dir, str(self.folder_node.uuid)) + self.assertEqual(os.listdir(out_dir), []) + + def test_node_repo_existing_file(self): + """Test 'verdi node repo cp' command with an existing file name.""" + + with tempfile.TemporaryDirectory() as tmp_dir: + file_content = 'some content' + out_dir = os.path.join(tmp_dir, str(self.folder_node.uuid)) + os.makedirs(out_dir) + with open(os.path.join(out_dir, self.filename_1), 'w') as file: + file.write(file_content) + options = [str(self.folder_node.uuid), tmp_dir] + result = self.cli_runner.invoke(cmd_node.repo_cp, options) + + self.assertIn('Warning', result.output) + self.assertEqual(sorted(os.listdir(out_dir)), sorted([self.filename_1, self.filename_2])) + with open(os.path.join(out_dir, self.filename_1), 'r') as file_1: + self.assertEqual(file_1.read(), file_content) + with open(os.path.join(out_dir, self.filename_2), 'r') as file_2: + self.assertEqual(file_2.read(), self.content_file2) + + def test_node_repo_existing_file_force(self): + """Test 'verdi node repo cp' command with an existing file name, forcing the overwrite.""" + + with tempfile.TemporaryDirectory() as tmp_dir: + file_content = 'some content' + out_dir = os.path.join(tmp_dir, str(self.folder_node.uuid)) + os.makedirs(out_dir) + with open(os.path.join(out_dir, self.filename_1), 'w') as file: + file.write(file_content) + options = ['--force', str(self.folder_node.uuid), tmp_dir] + self.cli_runner.invoke(cmd_node.repo_cp, options) + + self.assertEqual(sorted(os.listdir(out_dir)), sorted([self.filename_1, self.filename_2])) + with open(os.path.join(out_dir, self.filename_1), 'r') as file_1: + self.assertEqual(file_1.read(), self.content_file1) + with open(os.path.join(out_dir, self.filename_2), 'r') as file_2: + self.assertEqual(file_2.read(), self.content_file2) + def delete_temporary_file(filepath): """ diff --git a/aiida/cmdline/commands/cmd_node.py b/aiida/cmdline/commands/cmd_node.py index 8d42050824..526ec6d8f1 100644 --- a/aiida/cmdline/commands/cmd_node.py +++ b/aiida/cmdline/commands/cmd_node.py @@ -9,6 +9,8 @@ ########################################################################### """`verdi node` command.""" +import pathlib + import click import tabulate @@ -61,6 +63,40 @@ def repo_ls(node, relative_path, color): echo.echo_critical(exception) +@verdi_node_repo.command('cp') +@arguments.NODE() +@click.argument('src_files', type=str, nargs=-1) +@click.argument( + 'output_directory', + type=click.Path(exists=True, file_okay=False, writable=True), + required=True, +) +@click.option('--no-uuid', flag_value=True, help='Do not prefix the output paths with the node uuid.') +@options.FORCE() +@with_dbenv() +def repo_cp(node, output_directory, src_files, no_uuid, force): + """Copy the repository files of a node to an output directory.""" + output_directory = pathlib.Path(output_directory) + if not no_uuid: + output_directory /= node.uuid + + output_directory.mkdir(exist_ok=True) + if not src_files: + src_files = node.list_object_names() + for file_path in src_files: + if file_path not in node.list_object_names(): + echo.echo_warning("No object named '{}' in node repository.".format(file_path)) + continue + output_file_path = output_directory / file_path + if not force and output_file_path.exists(): + echo.echo_warning("Not copying '{}': File exists. Use '--force' option to override.".format(file_path)) + continue + with output_file_path.open(mode='wb') as out_f: + with node.open(file_path, 'rb') as in_f: + out_f.write(in_f.read()) + echo.echo("Outputs written to '{}'.".format(output_directory)) + + @verdi_node.command('label') @arguments.NODES() @options.LABEL(help='Set LABEL as the new label for all NODES') From 44413d8dbdf2c16cc98f2bf1bdcd8f059a08c9e4 Mon Sep 17 00:00:00 2001 From: Dominik Gresch Date: Tue, 10 Dec 2019 12:45:29 +0100 Subject: [PATCH 02/14] Change name and options in 'file copying' command. The command is renamed to 'verdi node repo dump'. The option of specifying specific file names to be copied is removed. The output paths no longer prefixed with a UUID. When a file or directory exists already, the command prompts to either abort, skip the current file / directory, overwrite the file / directory, or (in the case of directories only) merge the contents. A 'force' option can be given to always overwrite files / directories. Tests are not yet updated to the changed interface, since we want to get feedback on that first. --- aiida/cmdline/commands/cmd_node.py | 90 +++++++++++++++++++++++------- 1 file changed, 69 insertions(+), 21 deletions(-) diff --git a/aiida/cmdline/commands/cmd_node.py b/aiida/cmdline/commands/cmd_node.py index 526ec6d8f1..2700bc092c 100644 --- a/aiida/cmdline/commands/cmd_node.py +++ b/aiida/cmdline/commands/cmd_node.py @@ -9,6 +9,7 @@ ########################################################################### """`verdi node` command.""" +import shutil import pathlib import click @@ -63,38 +64,85 @@ def repo_ls(node, relative_path, color): echo.echo_critical(exception) -@verdi_node_repo.command('cp') +@verdi_node_repo.command('dump') +@click.option( + '-f', + '--force', + help='Overwrite existing directories and files without prompting for confirmation.', + flag_value=True +) @arguments.NODE() -@click.argument('src_files', type=str, nargs=-1) @click.argument( 'output_directory', type=click.Path(exists=True, file_okay=False, writable=True), required=True, ) -@click.option('--no-uuid', flag_value=True, help='Do not prefix the output paths with the node uuid.') -@options.FORCE() @with_dbenv() -def repo_cp(node, output_directory, src_files, no_uuid, force): +def repo_dump(node, output_directory, force): """Copy the repository files of a node to an output directory.""" + from aiida.orm.utils.repository import FileType + output_directory = pathlib.Path(output_directory) - if not no_uuid: - output_directory /= node.uuid output_directory.mkdir(exist_ok=True) - if not src_files: - src_files = node.list_object_names() - for file_path in src_files: - if file_path not in node.list_object_names(): - echo.echo_warning("No object named '{}' in node repository.".format(file_path)) - continue - output_file_path = output_directory / file_path - if not force and output_file_path.exists(): - echo.echo_warning("Not copying '{}': File exists. Use '--force' option to override.".format(file_path)) - continue - with output_file_path.open(mode='wb') as out_f: - with node.open(file_path, 'rb') as in_f: - out_f.write(in_f.read()) - echo.echo("Outputs written to '{}'.".format(output_directory)) + + def _copy_tree(key, output_dir): # pylint: disable=too-many-branches + for file in node.list_objects(key=key): + if not key: + file_path = file.name + else: + file_path = key + '/' + file.name + if file.type == FileType.DIRECTORY: + new_out_dir = output_dir / file.name + if new_out_dir.exists(): + if force: + prompt_res = 'o' + else: + prompt_res = click.prompt( + "Directory '{}' exists. Do you wish to abort [a], " + 'skip this directory [s], merge directory contents [m], ' + 'or overwrite the directory (delete current contents) [o]?'.format(new_out_dir), + default='a', + type=click.Choice(['a', 's', 'm', 'o']) + ) + if prompt_res == 'a': + raise click.Abort + elif prompt_res == 's': + continue + elif prompt_res == 'o': + shutil.rmtree(new_out_dir) + new_out_dir.mkdir() + else: + assert prompt_res == 'm' + else: + new_out_dir.mkdir() + + _copy_tree(key=file_path, output_dir=new_out_dir) + else: + out_file_path = (output_dir / file.name) + if out_file_path.exists(): + if force: + prompt_res = 'o' + else: + prompt_res = click.prompt( + 'File {} exists. Do you wish to abort [a], skip this file [s], or overwrite the file [o]?'. + format(out_file_path), + default='a', + type=click.Choice(['a', 's', 'o']) + ) + + if prompt_res == 'a': + raise click.Abort + elif prompt_res == 's': + continue + else: + assert prompt_res == 'o' + out_file_path.unlink() + with node.open(file_path, 'rb') as in_file: + with out_file_path.open('wb') as out_file: + out_file.write(in_file.read()) + + _copy_tree(key='', output_dir=output_directory) @verdi_node.command('label') From 6bd8b2a1901aa7ee74e7b333fcb65e8a03f68aca Mon Sep 17 00:00:00 2001 From: Dominik Gresch Date: Tue, 10 Dec 2019 15:44:39 +0100 Subject: [PATCH 03/14] Apply suggestions for simplified prompt wording. Apply suggestions for simplified wording of the prompts that ask if a file / directory should be replaced / merged / skipped, or the operation should be aborted. Co-Authored-By: Leopold Talirz --- aiida/cmdline/commands/cmd_node.py | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/aiida/cmdline/commands/cmd_node.py b/aiida/cmdline/commands/cmd_node.py index 2700bc092c..d29dd905f6 100644 --- a/aiida/cmdline/commands/cmd_node.py +++ b/aiida/cmdline/commands/cmd_node.py @@ -66,10 +66,7 @@ def repo_ls(node, relative_path, color): @verdi_node_repo.command('dump') @click.option( - '-f', - '--force', - help='Overwrite existing directories and files without prompting for confirmation.', - flag_value=True + '-f', '--force', help='Replace existing directories and files without prompting for confirmation.', flag_value=True ) @arguments.NODE() @click.argument( @@ -96,20 +93,20 @@ def _copy_tree(key, output_dir): # pylint: disable=too-many-branches new_out_dir = output_dir / file.name if new_out_dir.exists(): if force: - prompt_res = 'o' + prompt_res = 'r' else: prompt_res = click.prompt( - "Directory '{}' exists. Do you wish to abort [a], " - 'skip this directory [s], merge directory contents [m], ' - 'or overwrite the directory (delete current contents) [o]?'.format(new_out_dir), + "Directory '{}' exists. Abort [a], " + 'skip [s], merge contents [m], ' + 'or replace [r]?'.format(new_out_dir), default='a', - type=click.Choice(['a', 's', 'm', 'o']) + type=click.Choice(['a', 's', 'm', 'r']) ) if prompt_res == 'a': raise click.Abort elif prompt_res == 's': continue - elif prompt_res == 'o': + elif prompt_res == 'r': shutil.rmtree(new_out_dir) new_out_dir.mkdir() else: @@ -125,10 +122,9 @@ def _copy_tree(key, output_dir): # pylint: disable=too-many-branches prompt_res = 'o' else: prompt_res = click.prompt( - 'File {} exists. Do you wish to abort [a], skip this file [s], or overwrite the file [o]?'. - format(out_file_path), + "File '{}' exists. Abort [a], skip [s], or replace [r]?".format(out_file_path), default='a', - type=click.Choice(['a', 's', 'o']) + type=click.Choice(['a', 's', 'r']) ) if prompt_res == 'a': @@ -136,7 +132,7 @@ def _copy_tree(key, output_dir): # pylint: disable=too-many-branches elif prompt_res == 's': continue else: - assert prompt_res == 'o' + assert prompt_res == 'r' out_file_path.unlink() with node.open(file_path, 'rb') as in_file: with out_file_path.open('wb') as out_file: From 2c18ba01a45351383c90d005ae04c7af7ef78397 Mon Sep 17 00:00:00 2001 From: Dominik Gresch Date: Mon, 9 Dec 2019 18:12:26 +0100 Subject: [PATCH 04/14] Add 'verdi node repo cp' command. The command takes a node pk / uuid, zero or more file names, and an ouput directory as arguments. By default, it will create a sub-directory with the node uuid in the output directory, and copy the files with the given names from the repository to that directory. If no file names are given, all objects in the repository are copied. The command has two optional flags: --no-uuid: copies the files directly into the output directory, without uuid prefix --force: Copy files even if they already exist at the target destination --- .../tests/cmdline/commands/test_node.py | 114 ++++++++++++++++++ aiida/cmdline/commands/cmd_node.py | 36 ++++++ 2 files changed, 150 insertions(+) diff --git a/aiida/backends/tests/cmdline/commands/test_node.py b/aiida/backends/tests/cmdline/commands/test_node.py index ac596008f7..629f2673ad 100644 --- a/aiida/backends/tests/cmdline/commands/test_node.py +++ b/aiida/backends/tests/cmdline/commands/test_node.py @@ -10,6 +10,7 @@ """Tests for verdi node""" import os +import io import errno import tempfile @@ -52,6 +53,17 @@ def setUpClass(cls, *args, **kwargs): cls.node = node + # Set up a FolderData for the node repo cp tests. + folder_node = orm.FolderData() + cls.content_file1 = 'nobody expects' + cls.content_file2 = 'the minister of silly walks' + cls.filename_1 = 'test.txt' + cls.filename_2 = 'else.txt' + folder_node.put_object_from_filelike(io.StringIO(cls.content_file1), key=cls.filename_1) + folder_node.put_object_from_filelike(io.StringIO(cls.content_file2), key=cls.filename_2) + folder_node.store() + cls.folder_node = folder_node + def setUp(self): self.cli_runner = CliRunner() @@ -171,6 +183,108 @@ def test_node_extras(self): result = self.cli_runner.invoke(cmd_node.extras, options) self.assertIsNone(result.exception, result.output) + def test_node_repo_cp(self): + """Test 'verdi node repo cp' command.""" + + with tempfile.TemporaryDirectory() as tmp_dir: + options = [str(self.folder_node.uuid), tmp_dir] + self.cli_runner.invoke(cmd_node.repo_cp, options) + + out_dir = os.path.join(tmp_dir, str(self.folder_node.uuid)) + self.assertEqual(sorted(os.listdir(out_dir)), sorted([self.filename_1, self.filename_2])) + with open(os.path.join(out_dir, self.filename_1), 'r') as file_1: + self.assertEqual(file_1.read(), self.content_file1) + with open(os.path.join(out_dir, self.filename_2), 'r') as file_2: + self.assertEqual(file_2.read(), self.content_file2) + + def test_node_repo_cp_no_uuid(self): + """Test 'verdi node repo cp' command with '--no-uuid' option.""" + + with tempfile.TemporaryDirectory() as tmp_dir: + options = [str(self.folder_node.uuid), '--no-uuid', tmp_dir] + self.cli_runner.invoke(cmd_node.repo_cp, options) + + self.assertEqual(sorted(os.listdir(tmp_dir)), sorted([self.filename_1, self.filename_2])) + with open(os.path.join(tmp_dir, self.filename_1), 'r') as file_1: + self.assertEqual(file_1.read(), self.content_file1) + with open(os.path.join(tmp_dir, self.filename_2), 'r') as file_2: + self.assertEqual(file_2.read(), self.content_file2) + + def test_node_repo_cp_one_file(self): + """Test 'verdi node repo cp' command with explicitly specified file name.""" + + with tempfile.TemporaryDirectory() as tmp_dir: + options = [str(self.folder_node.uuid), self.filename_1, tmp_dir] + self.cli_runner.invoke(cmd_node.repo_cp, options) + + out_dir = os.path.join(tmp_dir, str(self.folder_node.uuid)) + self.assertEqual(os.listdir(out_dir), [self.filename_1]) + with open(os.path.join(out_dir, self.filename_1), 'r') as file_1: + self.assertEqual(file_1.read(), self.content_file1) + + def test_node_repo_cp_two_files_explicit(self): + """Test 'verdi node repo cp' command with two explicitly specified file names.""" + + with tempfile.TemporaryDirectory() as tmp_dir: + options = [str(self.folder_node.uuid), self.filename_2, self.filename_1, tmp_dir] + self.cli_runner.invoke(cmd_node.repo_cp, options) + + out_dir = os.path.join(tmp_dir, str(self.folder_node.uuid)) + self.assertEqual(sorted(os.listdir(out_dir)), sorted([self.filename_1, self.filename_2])) + with open(os.path.join(out_dir, self.filename_1), 'r') as file_1: + self.assertEqual(file_1.read(), self.content_file1) + with open(os.path.join(out_dir, self.filename_2), 'r') as file_2: + self.assertEqual(file_2.read(), self.content_file2) + + def test_node_repo_inexistent(self): + """Test 'verdi node repo cp' command with an inexistent file name.""" + + with tempfile.TemporaryDirectory() as tmp_dir: + options = [str(self.folder_node.uuid), 'inexistent_filename', tmp_dir] + result = self.cli_runner.invoke(cmd_node.repo_cp, options) + + self.assertIn('Warning', result.output) + + out_dir = os.path.join(tmp_dir, str(self.folder_node.uuid)) + self.assertEqual(os.listdir(out_dir), []) + + def test_node_repo_existing_file(self): + """Test 'verdi node repo cp' command with an existing file name.""" + + with tempfile.TemporaryDirectory() as tmp_dir: + file_content = 'some content' + out_dir = os.path.join(tmp_dir, str(self.folder_node.uuid)) + os.makedirs(out_dir) + with open(os.path.join(out_dir, self.filename_1), 'w') as file: + file.write(file_content) + options = [str(self.folder_node.uuid), tmp_dir] + result = self.cli_runner.invoke(cmd_node.repo_cp, options) + + self.assertIn('Warning', result.output) + self.assertEqual(sorted(os.listdir(out_dir)), sorted([self.filename_1, self.filename_2])) + with open(os.path.join(out_dir, self.filename_1), 'r') as file_1: + self.assertEqual(file_1.read(), file_content) + with open(os.path.join(out_dir, self.filename_2), 'r') as file_2: + self.assertEqual(file_2.read(), self.content_file2) + + def test_node_repo_existing_file_force(self): + """Test 'verdi node repo cp' command with an existing file name, forcing the overwrite.""" + + with tempfile.TemporaryDirectory() as tmp_dir: + file_content = 'some content' + out_dir = os.path.join(tmp_dir, str(self.folder_node.uuid)) + os.makedirs(out_dir) + with open(os.path.join(out_dir, self.filename_1), 'w') as file: + file.write(file_content) + options = ['--force', str(self.folder_node.uuid), tmp_dir] + self.cli_runner.invoke(cmd_node.repo_cp, options) + + self.assertEqual(sorted(os.listdir(out_dir)), sorted([self.filename_1, self.filename_2])) + with open(os.path.join(out_dir, self.filename_1), 'r') as file_1: + self.assertEqual(file_1.read(), self.content_file1) + with open(os.path.join(out_dir, self.filename_2), 'r') as file_2: + self.assertEqual(file_2.read(), self.content_file2) + def delete_temporary_file(filepath): """ diff --git a/aiida/cmdline/commands/cmd_node.py b/aiida/cmdline/commands/cmd_node.py index 8d42050824..526ec6d8f1 100644 --- a/aiida/cmdline/commands/cmd_node.py +++ b/aiida/cmdline/commands/cmd_node.py @@ -9,6 +9,8 @@ ########################################################################### """`verdi node` command.""" +import pathlib + import click import tabulate @@ -61,6 +63,40 @@ def repo_ls(node, relative_path, color): echo.echo_critical(exception) +@verdi_node_repo.command('cp') +@arguments.NODE() +@click.argument('src_files', type=str, nargs=-1) +@click.argument( + 'output_directory', + type=click.Path(exists=True, file_okay=False, writable=True), + required=True, +) +@click.option('--no-uuid', flag_value=True, help='Do not prefix the output paths with the node uuid.') +@options.FORCE() +@with_dbenv() +def repo_cp(node, output_directory, src_files, no_uuid, force): + """Copy the repository files of a node to an output directory.""" + output_directory = pathlib.Path(output_directory) + if not no_uuid: + output_directory /= node.uuid + + output_directory.mkdir(exist_ok=True) + if not src_files: + src_files = node.list_object_names() + for file_path in src_files: + if file_path not in node.list_object_names(): + echo.echo_warning("No object named '{}' in node repository.".format(file_path)) + continue + output_file_path = output_directory / file_path + if not force and output_file_path.exists(): + echo.echo_warning("Not copying '{}': File exists. Use '--force' option to override.".format(file_path)) + continue + with output_file_path.open(mode='wb') as out_f: + with node.open(file_path, 'rb') as in_f: + out_f.write(in_f.read()) + echo.echo("Outputs written to '{}'.".format(output_directory)) + + @verdi_node.command('label') @arguments.NODES() @options.LABEL(help='Set LABEL as the new label for all NODES') From 27ed4828aa67d90666e34bba437f0e749a3b2806 Mon Sep 17 00:00:00 2001 From: Dominik Gresch Date: Tue, 10 Dec 2019 12:45:29 +0100 Subject: [PATCH 05/14] Change name and options in 'file copying' command. The command is renamed to 'verdi node repo dump'. The option of specifying specific file names to be copied is removed. The output paths no longer prefixed with a UUID. When a file or directory exists already, the command prompts to either abort, skip the current file / directory, overwrite the file / directory, or (in the case of directories only) merge the contents. A 'force' option can be given to always overwrite files / directories. Tests are not yet updated to the changed interface, since we want to get feedback on that first. --- aiida/cmdline/commands/cmd_node.py | 90 +++++++++++++++++++++++------- 1 file changed, 69 insertions(+), 21 deletions(-) diff --git a/aiida/cmdline/commands/cmd_node.py b/aiida/cmdline/commands/cmd_node.py index 526ec6d8f1..2700bc092c 100644 --- a/aiida/cmdline/commands/cmd_node.py +++ b/aiida/cmdline/commands/cmd_node.py @@ -9,6 +9,7 @@ ########################################################################### """`verdi node` command.""" +import shutil import pathlib import click @@ -63,38 +64,85 @@ def repo_ls(node, relative_path, color): echo.echo_critical(exception) -@verdi_node_repo.command('cp') +@verdi_node_repo.command('dump') +@click.option( + '-f', + '--force', + help='Overwrite existing directories and files without prompting for confirmation.', + flag_value=True +) @arguments.NODE() -@click.argument('src_files', type=str, nargs=-1) @click.argument( 'output_directory', type=click.Path(exists=True, file_okay=False, writable=True), required=True, ) -@click.option('--no-uuid', flag_value=True, help='Do not prefix the output paths with the node uuid.') -@options.FORCE() @with_dbenv() -def repo_cp(node, output_directory, src_files, no_uuid, force): +def repo_dump(node, output_directory, force): """Copy the repository files of a node to an output directory.""" + from aiida.orm.utils.repository import FileType + output_directory = pathlib.Path(output_directory) - if not no_uuid: - output_directory /= node.uuid output_directory.mkdir(exist_ok=True) - if not src_files: - src_files = node.list_object_names() - for file_path in src_files: - if file_path not in node.list_object_names(): - echo.echo_warning("No object named '{}' in node repository.".format(file_path)) - continue - output_file_path = output_directory / file_path - if not force and output_file_path.exists(): - echo.echo_warning("Not copying '{}': File exists. Use '--force' option to override.".format(file_path)) - continue - with output_file_path.open(mode='wb') as out_f: - with node.open(file_path, 'rb') as in_f: - out_f.write(in_f.read()) - echo.echo("Outputs written to '{}'.".format(output_directory)) + + def _copy_tree(key, output_dir): # pylint: disable=too-many-branches + for file in node.list_objects(key=key): + if not key: + file_path = file.name + else: + file_path = key + '/' + file.name + if file.type == FileType.DIRECTORY: + new_out_dir = output_dir / file.name + if new_out_dir.exists(): + if force: + prompt_res = 'o' + else: + prompt_res = click.prompt( + "Directory '{}' exists. Do you wish to abort [a], " + 'skip this directory [s], merge directory contents [m], ' + 'or overwrite the directory (delete current contents) [o]?'.format(new_out_dir), + default='a', + type=click.Choice(['a', 's', 'm', 'o']) + ) + if prompt_res == 'a': + raise click.Abort + elif prompt_res == 's': + continue + elif prompt_res == 'o': + shutil.rmtree(new_out_dir) + new_out_dir.mkdir() + else: + assert prompt_res == 'm' + else: + new_out_dir.mkdir() + + _copy_tree(key=file_path, output_dir=new_out_dir) + else: + out_file_path = (output_dir / file.name) + if out_file_path.exists(): + if force: + prompt_res = 'o' + else: + prompt_res = click.prompt( + 'File {} exists. Do you wish to abort [a], skip this file [s], or overwrite the file [o]?'. + format(out_file_path), + default='a', + type=click.Choice(['a', 's', 'o']) + ) + + if prompt_res == 'a': + raise click.Abort + elif prompt_res == 's': + continue + else: + assert prompt_res == 'o' + out_file_path.unlink() + with node.open(file_path, 'rb') as in_file: + with out_file_path.open('wb') as out_file: + out_file.write(in_file.read()) + + _copy_tree(key='', output_dir=output_directory) @verdi_node.command('label') From b63281ec3887570082a0af6b77b477c7cf0e6fc3 Mon Sep 17 00:00:00 2001 From: Dominik Gresch Date: Tue, 10 Dec 2019 15:44:39 +0100 Subject: [PATCH 06/14] Apply suggestions for simplified prompt wording. Apply suggestions for simplified wording of the prompts that ask if a file / directory should be replaced / merged / skipped, or the operation should be aborted. Co-Authored-By: Leopold Talirz --- aiida/cmdline/commands/cmd_node.py | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/aiida/cmdline/commands/cmd_node.py b/aiida/cmdline/commands/cmd_node.py index 2700bc092c..d29dd905f6 100644 --- a/aiida/cmdline/commands/cmd_node.py +++ b/aiida/cmdline/commands/cmd_node.py @@ -66,10 +66,7 @@ def repo_ls(node, relative_path, color): @verdi_node_repo.command('dump') @click.option( - '-f', - '--force', - help='Overwrite existing directories and files without prompting for confirmation.', - flag_value=True + '-f', '--force', help='Replace existing directories and files without prompting for confirmation.', flag_value=True ) @arguments.NODE() @click.argument( @@ -96,20 +93,20 @@ def _copy_tree(key, output_dir): # pylint: disable=too-many-branches new_out_dir = output_dir / file.name if new_out_dir.exists(): if force: - prompt_res = 'o' + prompt_res = 'r' else: prompt_res = click.prompt( - "Directory '{}' exists. Do you wish to abort [a], " - 'skip this directory [s], merge directory contents [m], ' - 'or overwrite the directory (delete current contents) [o]?'.format(new_out_dir), + "Directory '{}' exists. Abort [a], " + 'skip [s], merge contents [m], ' + 'or replace [r]?'.format(new_out_dir), default='a', - type=click.Choice(['a', 's', 'm', 'o']) + type=click.Choice(['a', 's', 'm', 'r']) ) if prompt_res == 'a': raise click.Abort elif prompt_res == 's': continue - elif prompt_res == 'o': + elif prompt_res == 'r': shutil.rmtree(new_out_dir) new_out_dir.mkdir() else: @@ -125,10 +122,9 @@ def _copy_tree(key, output_dir): # pylint: disable=too-many-branches prompt_res = 'o' else: prompt_res = click.prompt( - 'File {} exists. Do you wish to abort [a], skip this file [s], or overwrite the file [o]?'. - format(out_file_path), + "File '{}' exists. Abort [a], skip [s], or replace [r]?".format(out_file_path), default='a', - type=click.Choice(['a', 's', 'o']) + type=click.Choice(['a', 's', 'r']) ) if prompt_res == 'a': @@ -136,7 +132,7 @@ def _copy_tree(key, output_dir): # pylint: disable=too-many-branches elif prompt_res == 's': continue else: - assert prompt_res == 'o' + assert prompt_res == 'r' out_file_path.unlink() with node.open(file_path, 'rb') as in_file: with out_file_path.open('wb') as out_file: From e616d7e988df512f86cab58c8f5d7e4f76dca84e Mon Sep 17 00:00:00 2001 From: Dominik Gresch Date: Tue, 10 Dec 2019 23:04:49 +0100 Subject: [PATCH 07/14] Adapt test cases to modified interface of the dump command. --- .../tests/cmdline/commands/test_node.py | 260 ++++++++++++------ 1 file changed, 180 insertions(+), 80 deletions(-) diff --git a/aiida/backends/tests/cmdline/commands/test_node.py b/aiida/backends/tests/cmdline/commands/test_node.py index 629f2673ad..87b228d54f 100644 --- a/aiida/backends/tests/cmdline/commands/test_node.py +++ b/aiida/backends/tests/cmdline/commands/test_node.py @@ -12,6 +12,7 @@ import os import io import errno +import pathlib import tempfile from click.testing import CliRunner @@ -57,10 +58,10 @@ def setUpClass(cls, *args, **kwargs): folder_node = orm.FolderData() cls.content_file1 = 'nobody expects' cls.content_file2 = 'the minister of silly walks' - cls.filename_1 = 'test.txt' - cls.filename_2 = 'else.txt' - folder_node.put_object_from_filelike(io.StringIO(cls.content_file1), key=cls.filename_1) - folder_node.put_object_from_filelike(io.StringIO(cls.content_file2), key=cls.filename_2) + cls.key_file1 = 'some/nested/folder/filename.txt' + cls.key_file2 = 'some_other_file.txt' + folder_node.put_object_from_filelike(io.StringIO(cls.content_file1), key=cls.key_file1) + folder_node.put_object_from_filelike(io.StringIO(cls.content_file2), key=cls.key_file2) folder_node.store() cls.folder_node = folder_node @@ -183,107 +184,206 @@ def test_node_extras(self): result = self.cli_runner.invoke(cmd_node.extras, options) self.assertIsNone(result.exception, result.output) - def test_node_repo_cp(self): - """Test 'verdi node repo cp' command.""" + def test_node_repo_dump(self): + """Test 'verdi node repo dump' command.""" with tempfile.TemporaryDirectory() as tmp_dir: options = [str(self.folder_node.uuid), tmp_dir] - self.cli_runner.invoke(cmd_node.repo_cp, options) - - out_dir = os.path.join(tmp_dir, str(self.folder_node.uuid)) - self.assertEqual(sorted(os.listdir(out_dir)), sorted([self.filename_1, self.filename_2])) - with open(os.path.join(out_dir, self.filename_1), 'r') as file_1: - self.assertEqual(file_1.read(), self.content_file1) - with open(os.path.join(out_dir, self.filename_2), 'r') as file_2: - self.assertEqual(file_2.read(), self.content_file2) - - def test_node_repo_cp_no_uuid(self): - """Test 'verdi node repo cp' command with '--no-uuid' option.""" + res = self.cli_runner.invoke(cmd_node.repo_dump, options) + self.assertFalse(res.stdout) + + for file_key, content in [(self.key_file1, self.content_file1), (self.key_file2, self.content_file2)]: + curr_path = pathlib.Path(tmp_dir) + for key_part in file_key.split('/'): + curr_path /= key_part + self.assertTrue(curr_path.exists()) + with curr_path.open('r') as res_file: + self.assertEqual(res_file.read(), content) + + def test_node_repo_dump_with_force(self): + """Test 'verdi node repo dump' command with --force option. + + This test first creates some other (clashing) content in the output + directory, but this should be replaced because of the '--force' option. + """ with tempfile.TemporaryDirectory() as tmp_dir: - options = [str(self.folder_node.uuid), '--no-uuid', tmp_dir] - self.cli_runner.invoke(cmd_node.repo_cp, options) + # First, create some other contents + sub_dir = pathlib.Path(tmp_dir) / self.key_file1.split('/')[0] + sub_dir.mkdir() + file_to_replace = (sub_dir / 'other_file') + with file_to_replace.open('w') as out_f: + out_f.write('ni!') - self.assertEqual(sorted(os.listdir(tmp_dir)), sorted([self.filename_1, self.filename_2])) - with open(os.path.join(tmp_dir, self.filename_1), 'r') as file_1: - self.assertEqual(file_1.read(), self.content_file1) - with open(os.path.join(tmp_dir, self.filename_2), 'r') as file_2: - self.assertEqual(file_2.read(), self.content_file2) + options = ['--force', str(self.folder_node.uuid), tmp_dir] + res = self.cli_runner.invoke(cmd_node.repo_dump, options) + self.assertFalse(res.stdout) - def test_node_repo_cp_one_file(self): - """Test 'verdi node repo cp' command with explicitly specified file name.""" + for file_key, content in [(self.key_file1, self.content_file1), (self.key_file2, self.content_file2)]: + curr_path = pathlib.Path(tmp_dir) + for key_part in file_key.split('/'): + curr_path /= key_part + self.assertTrue(curr_path.exists()) + with curr_path.open('r') as res_file: + self.assertEqual(res_file.read(), content) - with tempfile.TemporaryDirectory() as tmp_dir: - options = [str(self.folder_node.uuid), self.filename_1, tmp_dir] - self.cli_runner.invoke(cmd_node.repo_cp, options) + self.assertFalse(file_to_replace.exists()) - out_dir = os.path.join(tmp_dir, str(self.folder_node.uuid)) - self.assertEqual(os.listdir(out_dir), [self.filename_1]) - with open(os.path.join(out_dir, self.filename_1), 'r') as file_1: - self.assertEqual(file_1.read(), self.content_file1) + def test_node_repo_dump_with_replace(self): + """Test 'verdi node repo dump' command with 'replace' prompt option. - def test_node_repo_cp_two_files_explicit(self): - """Test 'verdi node repo cp' command with two explicitly specified file names.""" + This test first creates some other (clashing) content in the output + directory, but this should be replaced because of the 'replace' option. + """ with tempfile.TemporaryDirectory() as tmp_dir: - options = [str(self.folder_node.uuid), self.filename_2, self.filename_1, tmp_dir] - self.cli_runner.invoke(cmd_node.repo_cp, options) + # First, create some other contents + sub_dir = pathlib.Path(tmp_dir) / self.key_file1.split('/')[0] + sub_dir.mkdir() + file_to_replace = (sub_dir / 'other_file') + with file_to_replace.open('w') as out_f: + out_f.write('ni!') - out_dir = os.path.join(tmp_dir, str(self.folder_node.uuid)) - self.assertEqual(sorted(os.listdir(out_dir)), sorted([self.filename_1, self.filename_2])) - with open(os.path.join(out_dir, self.filename_1), 'r') as file_1: - self.assertEqual(file_1.read(), self.content_file1) - with open(os.path.join(out_dir, self.filename_2), 'r') as file_2: - self.assertEqual(file_2.read(), self.content_file2) + options = [str(self.folder_node.uuid), tmp_dir] + res = self.cli_runner.invoke(cmd_node.repo_dump, options, input='r\n') + self.assertIn('Directory', res.stdout) + self.assertIn('exists', res.stdout) + + for file_key, content in [(self.key_file1, self.content_file1), (self.key_file2, self.content_file2)]: + curr_path = pathlib.Path(tmp_dir) + for key_part in file_key.split('/'): + curr_path /= key_part + self.assertTrue(curr_path.exists()) + with curr_path.open('r') as res_file: + self.assertEqual(res_file.read(), content) + + self.assertFalse(file_to_replace.exists()) + + def test_node_repo_dump_with_replace_file(self): + """Test 'verdi node repo dump' command with 'replace' prompt option on a file. + + This test first creates some other (clashing) content in the output + directory, but this should be replaced because of the 'replace' option. In contrast + to the other test, here the 'replace' called at the level where the files clash, + not the directories. + """ - def test_node_repo_inexistent(self): - """Test 'verdi node repo cp' command with an inexistent file name.""" + with tempfile.TemporaryDirectory() as tmp_dir: + # First, create some other contents + file_to_replace = pathlib.Path(tmp_dir) / self.key_file2 + with file_to_replace.open('w') as out_f: + out_f.write('ni!') + + options = [str(self.folder_node.uuid), tmp_dir] + res = self.cli_runner.invoke(cmd_node.repo_dump, options, input='r\n') + self.assertIn('File', res.stdout) + self.assertIn('exists', res.stdout) + + for file_key, content in [(self.key_file1, self.content_file1), (self.key_file2, self.content_file2)]: + curr_path = pathlib.Path(tmp_dir) + for key_part in file_key.split('/'): + curr_path /= key_part + self.assertTrue(curr_path.exists()) + with curr_path.open('r') as res_file: + self.assertEqual(res_file.read(), content) + + def test_node_repo_dump_with_abort(self): + """Test 'verdi node repo dump' command with the 'abort' prompt option. + + This test first sets up another file with the same directory as where + 'file1' should be placed. Because 'abort' is selected, it should + create neither of the two files, and the directory should be left + as is. + """ with tempfile.TemporaryDirectory() as tmp_dir: - options = [str(self.folder_node.uuid), 'inexistent_filename', tmp_dir] - result = self.cli_runner.invoke(cmd_node.repo_cp, options) + # First, create some other contents + sub_dir = pathlib.Path(tmp_dir) / self.key_file1.split('/')[0] + sub_dir.mkdir() + file_not_to_replace = (sub_dir / 'other_file') + with file_not_to_replace.open('w') as out_f: + out_f.write('ni!') - self.assertIn('Warning', result.output) + options = [str(self.folder_node.uuid), tmp_dir] + res = self.cli_runner.invoke(cmd_node.repo_dump, options, input='a\n') + self.assertIn('Directory', res.stdout) + self.assertIn('exists', res.stdout) + self.assertIn('Aborted!', res.stdout) - out_dir = os.path.join(tmp_dir, str(self.folder_node.uuid)) - self.assertEqual(os.listdir(out_dir), []) + for file_key in [self.key_file1, self.key_file2]: + curr_path = pathlib.Path(tmp_dir) - def test_node_repo_existing_file(self): - """Test 'verdi node repo cp' command with an existing file name.""" + for key_part in file_key.split('/'): + curr_path /= key_part + if curr_path not in [sub_dir, file_not_to_replace]: + self.assertFalse(curr_path.exists()) - with tempfile.TemporaryDirectory() as tmp_dir: - file_content = 'some content' - out_dir = os.path.join(tmp_dir, str(self.folder_node.uuid)) - os.makedirs(out_dir) - with open(os.path.join(out_dir, self.filename_1), 'w') as file: - file.write(file_content) - options = [str(self.folder_node.uuid), tmp_dir] - result = self.cli_runner.invoke(cmd_node.repo_cp, options) + self.assertTrue(file_not_to_replace.exists()) + + def test_node_repo_dump_with_skip(self): + """Test 'verdi node repo dump' command with the 'skip' prompt option.""" - self.assertIn('Warning', result.output) - self.assertEqual(sorted(os.listdir(out_dir)), sorted([self.filename_1, self.filename_2])) - with open(os.path.join(out_dir, self.filename_1), 'r') as file_1: - self.assertEqual(file_1.read(), file_content) - with open(os.path.join(out_dir, self.filename_2), 'r') as file_2: - self.assertEqual(file_2.read(), self.content_file2) + with tempfile.TemporaryDirectory() as tmp_dir: + # First, create some other contents + sub_dir = pathlib.Path(tmp_dir) / self.key_file1.split('/')[0] + sub_dir.mkdir() + file_not_to_replace = (sub_dir / 'other_file') + with file_not_to_replace.open('w') as out_f: + out_f.write('ni!') - def test_node_repo_existing_file_force(self): - """Test 'verdi node repo cp' command with an existing file name, forcing the overwrite.""" + options = [str(self.folder_node.uuid), tmp_dir] + res = self.cli_runner.invoke(cmd_node.repo_dump, options, input='s\n') + self.assertIn('Directory', res.stdout) + self.assertIn('exists', res.stdout) + + for file_key in [self.key_file1]: + curr_path = pathlib.Path(tmp_dir) + + for key_part in file_key.split('/'): + curr_path /= key_part + if curr_path not in [sub_dir, file_not_to_replace]: + self.assertFalse(curr_path.exists()) + + for file_key, content in [(self.key_file2, self.content_file2)]: + curr_path = pathlib.Path(tmp_dir) + for key_part in file_key.split('/'): + curr_path /= key_part + self.assertTrue(curr_path.exists()) + with curr_path.open('r') as res_file: + self.assertEqual(res_file.read(), content) + + self.assertTrue(file_not_to_replace.exists()) + + def test_node_repo_dump_with_merge(self): + """Test 'verdi node repo dump' command with the 'merge' prompt option. + + This test creates another file in the same (top-level) directory as + 'file1', and they should both exist in the end due to the 'merge' + prompt option. + """ with tempfile.TemporaryDirectory() as tmp_dir: - file_content = 'some content' - out_dir = os.path.join(tmp_dir, str(self.folder_node.uuid)) - os.makedirs(out_dir) - with open(os.path.join(out_dir, self.filename_1), 'w') as file: - file.write(file_content) - options = ['--force', str(self.folder_node.uuid), tmp_dir] - self.cli_runner.invoke(cmd_node.repo_cp, options) + # First, create some other contents + sub_dir = pathlib.Path(tmp_dir) / self.key_file1.split('/')[0] + sub_dir.mkdir() + file_not_to_replace = (sub_dir / 'other_file') + with file_not_to_replace.open('w') as out_f: + out_f.write('ni!') - self.assertEqual(sorted(os.listdir(out_dir)), sorted([self.filename_1, self.filename_2])) - with open(os.path.join(out_dir, self.filename_1), 'r') as file_1: - self.assertEqual(file_1.read(), self.content_file1) - with open(os.path.join(out_dir, self.filename_2), 'r') as file_2: - self.assertEqual(file_2.read(), self.content_file2) + options = [str(self.folder_node.uuid), tmp_dir] + res = self.cli_runner.invoke(cmd_node.repo_dump, options, input='m\n') + self.assertIn('Directory', res.stdout) + self.assertIn('exists', res.stdout) + + for file_key, content in [(self.key_file1, self.content_file1), (self.key_file2, self.content_file2)]: + curr_path = pathlib.Path(tmp_dir) + for key_part in file_key.split('/'): + curr_path /= key_part + self.assertTrue(curr_path.exists()) + with curr_path.open('r') as res_file: + self.assertEqual(res_file.read(), content) + + self.assertTrue(file_not_to_replace.exists()) def delete_temporary_file(filepath): From 8829b46766e8366f612814c37594dd67c374e224 Mon Sep 17 00:00:00 2001 From: Dominik Gresch Date: Wed, 11 Dec 2019 16:37:28 +0100 Subject: [PATCH 08/14] Add check that stderr is empty in 'node repo dump' tests. --- .../tests/cmdline/commands/test_node.py | 30 ++++++++++++++++--- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/aiida/backends/tests/cmdline/commands/test_node.py b/aiida/backends/tests/cmdline/commands/test_node.py index 87b228d54f..8f91c32e74 100644 --- a/aiida/backends/tests/cmdline/commands/test_node.py +++ b/aiida/backends/tests/cmdline/commands/test_node.py @@ -14,6 +14,7 @@ import errno import pathlib import tempfile +import contextlib from click.testing import CliRunner @@ -189,8 +190,11 @@ def test_node_repo_dump(self): with tempfile.TemporaryDirectory() as tmp_dir: options = [str(self.folder_node.uuid), tmp_dir] - res = self.cli_runner.invoke(cmd_node.repo_dump, options) + res = self.cli_runner.invoke(cmd_node.repo_dump, options, mix_stderr=False) self.assertFalse(res.stdout) + # If there was no stderr, accessing the attribute raises ValueError. + with contextlib.suppress(ValueError): + self.assertFalse(res.stderr) for file_key, content in [(self.key_file1, self.content_file1), (self.key_file2, self.content_file2)]: curr_path = pathlib.Path(tmp_dir) @@ -216,8 +220,11 @@ def test_node_repo_dump_with_force(self): out_f.write('ni!') options = ['--force', str(self.folder_node.uuid), tmp_dir] - res = self.cli_runner.invoke(cmd_node.repo_dump, options) + res = self.cli_runner.invoke(cmd_node.repo_dump, options, mix_stderr=False) self.assertFalse(res.stdout) + # If there was no stderr, accessing the attribute raises ValueError. + with contextlib.suppress(ValueError): + self.assertFalse(res.stderr) for file_key, content in [(self.key_file1, self.content_file1), (self.key_file2, self.content_file2)]: curr_path = pathlib.Path(tmp_dir) @@ -245,7 +252,10 @@ def test_node_repo_dump_with_replace(self): out_f.write('ni!') options = [str(self.folder_node.uuid), tmp_dir] - res = self.cli_runner.invoke(cmd_node.repo_dump, options, input='r\n') + res = self.cli_runner.invoke(cmd_node.repo_dump, options, input='r\n', mix_stderr=False) + # If there was no stderr, accessing the attribute raises ValueError. + with contextlib.suppress(ValueError): + self.assertFalse(res.stderr) self.assertIn('Directory', res.stdout) self.assertIn('exists', res.stdout) @@ -275,7 +285,10 @@ def test_node_repo_dump_with_replace_file(self): out_f.write('ni!') options = [str(self.folder_node.uuid), tmp_dir] - res = self.cli_runner.invoke(cmd_node.repo_dump, options, input='r\n') + res = self.cli_runner.invoke(cmd_node.repo_dump, options, input='r\n', mix_stderr=False) + # If there was no stderr, accessing the attribute raises ValueError. + with contextlib.suppress(ValueError): + self.assertFalse(res.stderr) self.assertIn('File', res.stdout) self.assertIn('exists', res.stdout) @@ -306,6 +319,9 @@ def test_node_repo_dump_with_abort(self): options = [str(self.folder_node.uuid), tmp_dir] res = self.cli_runner.invoke(cmd_node.repo_dump, options, input='a\n') + # If there was no stderr, accessing the attribute raises ValueError. + with contextlib.suppress(ValueError): + self.assertFalse(res.stderr) self.assertIn('Directory', res.stdout) self.assertIn('exists', res.stdout) self.assertIn('Aborted!', res.stdout) @@ -333,6 +349,9 @@ def test_node_repo_dump_with_skip(self): options = [str(self.folder_node.uuid), tmp_dir] res = self.cli_runner.invoke(cmd_node.repo_dump, options, input='s\n') + # If there was no stderr, accessing the attribute raises ValueError. + with contextlib.suppress(ValueError): + self.assertFalse(res.stderr) self.assertIn('Directory', res.stdout) self.assertIn('exists', res.stdout) @@ -372,6 +391,9 @@ def test_node_repo_dump_with_merge(self): options = [str(self.folder_node.uuid), tmp_dir] res = self.cli_runner.invoke(cmd_node.repo_dump, options, input='m\n') + # If there was no stderr, accessing the attribute raises ValueError. + with contextlib.suppress(ValueError): + self.assertFalse(res.stderr) self.assertIn('Directory', res.stdout) self.assertIn('exists', res.stdout) From d9ff8accbeedee25c42125894f9ed9fee189a1f5 Mon Sep 17 00:00:00 2001 From: Dominik Gresch Date: Wed, 11 Dec 2019 17:45:51 +0100 Subject: [PATCH 09/14] Fix and simplify '--force' handling, add test. The code was currently broken because not all flags were changed from 'o' to 'r' (in the rename from 'overwrite' to 'replace'). Adds a test which runs with '--force' where a single file is replaced. --- .../tests/cmdline/commands/test_node.py | 31 ++++++++++++++++++- aiida/cmdline/commands/cmd_node.py | 11 +++---- 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/aiida/backends/tests/cmdline/commands/test_node.py b/aiida/backends/tests/cmdline/commands/test_node.py index 8f91c32e74..5222ebc3d4 100644 --- a/aiida/backends/tests/cmdline/commands/test_node.py +++ b/aiida/backends/tests/cmdline/commands/test_node.py @@ -205,7 +205,7 @@ def test_node_repo_dump(self): self.assertEqual(res_file.read(), content) def test_node_repo_dump_with_force(self): - """Test 'verdi node repo dump' command with --force option. + """Test 'verdi node repo dump' command with --force option, where a directory clashes. This test first creates some other (clashing) content in the output directory, but this should be replaced because of the '--force' option. @@ -236,6 +236,35 @@ def test_node_repo_dump_with_force(self): self.assertFalse(file_to_replace.exists()) + def test_node_repo_dump_with_force_file(self): + """Test 'verdi node repo dump' command with --force option, where a file clashes. + + This test first creates a clashing file in the output + directory, but this should be replaced because of the '--force' option. + """ + + with tempfile.TemporaryDirectory() as tmp_dir: + # First, create some other contents + file_to_replace = pathlib.Path(tmp_dir) / self.key_file1 + file_to_replace.parent.mkdir(parents=True) + with file_to_replace.open('w') as out_f: + out_f.write('ni!') + + options = ['--force', str(self.folder_node.uuid), tmp_dir] + res = self.cli_runner.invoke(cmd_node.repo_dump, options, mix_stderr=False) + self.assertFalse(res.stdout) + # If there was no stderr, accessing the attribute raises ValueError. + with contextlib.suppress(ValueError): + self.assertFalse(res.stderr) + + for file_key, content in [(self.key_file1, self.content_file1), (self.key_file2, self.content_file2)]: + curr_path = pathlib.Path(tmp_dir) + for key_part in file_key.split('/'): + curr_path /= key_part + self.assertTrue(curr_path.exists()) + with curr_path.open('r') as res_file: + self.assertEqual(res_file.read(), content) + def test_node_repo_dump_with_replace(self): """Test 'verdi node repo dump' command with 'replace' prompt option. diff --git a/aiida/cmdline/commands/cmd_node.py b/aiida/cmdline/commands/cmd_node.py index d29dd905f6..b11a04c3a4 100644 --- a/aiida/cmdline/commands/cmd_node.py +++ b/aiida/cmdline/commands/cmd_node.py @@ -84,6 +84,8 @@ def repo_dump(node, output_directory, force): output_directory.mkdir(exist_ok=True) def _copy_tree(key, output_dir): # pylint: disable=too-many-branches + if force: + prompt_res = 'r' for file in node.list_objects(key=key): if not key: file_path = file.name @@ -92,9 +94,7 @@ def _copy_tree(key, output_dir): # pylint: disable=too-many-branches if file.type == FileType.DIRECTORY: new_out_dir = output_dir / file.name if new_out_dir.exists(): - if force: - prompt_res = 'r' - else: + if not force: prompt_res = click.prompt( "Directory '{}' exists. Abort [a], " 'skip [s], merge contents [m], ' @@ -116,11 +116,10 @@ def _copy_tree(key, output_dir): # pylint: disable=too-many-branches _copy_tree(key=file_path, output_dir=new_out_dir) else: + assert file.type == FileType.FILE out_file_path = (output_dir / file.name) if out_file_path.exists(): - if force: - prompt_res = 'o' - else: + if not force: prompt_res = click.prompt( "File '{}' exists. Abort [a], skip [s], or replace [r]?".format(out_file_path), default='a', From da7ab3fa1fcde2b5c0f645e9ebfd26130c536563 Mon Sep 17 00:00:00 2001 From: Dominik Gresch Date: Wed, 11 Dec 2019 17:55:56 +0100 Subject: [PATCH 10/14] Do not catch exceptions when running commands through click runner. --- .../tests/cmdline/commands/test_node.py | 49 ++++++------------- 1 file changed, 14 insertions(+), 35 deletions(-) diff --git a/aiida/backends/tests/cmdline/commands/test_node.py b/aiida/backends/tests/cmdline/commands/test_node.py index 5222ebc3d4..d9bd62c652 100644 --- a/aiida/backends/tests/cmdline/commands/test_node.py +++ b/aiida/backends/tests/cmdline/commands/test_node.py @@ -14,7 +14,6 @@ import errno import pathlib import tempfile -import contextlib from click.testing import CliRunner @@ -190,11 +189,8 @@ def test_node_repo_dump(self): with tempfile.TemporaryDirectory() as tmp_dir: options = [str(self.folder_node.uuid), tmp_dir] - res = self.cli_runner.invoke(cmd_node.repo_dump, options, mix_stderr=False) + res = self.cli_runner.invoke(cmd_node.repo_dump, options, catch_exceptions=False) self.assertFalse(res.stdout) - # If there was no stderr, accessing the attribute raises ValueError. - with contextlib.suppress(ValueError): - self.assertFalse(res.stderr) for file_key, content in [(self.key_file1, self.content_file1), (self.key_file2, self.content_file2)]: curr_path = pathlib.Path(tmp_dir) @@ -220,11 +216,8 @@ def test_node_repo_dump_with_force(self): out_f.write('ni!') options = ['--force', str(self.folder_node.uuid), tmp_dir] - res = self.cli_runner.invoke(cmd_node.repo_dump, options, mix_stderr=False) + res = self.cli_runner.invoke(cmd_node.repo_dump, options, catch_exceptions=False) self.assertFalse(res.stdout) - # If there was no stderr, accessing the attribute raises ValueError. - with contextlib.suppress(ValueError): - self.assertFalse(res.stderr) for file_key, content in [(self.key_file1, self.content_file1), (self.key_file2, self.content_file2)]: curr_path = pathlib.Path(tmp_dir) @@ -245,17 +238,13 @@ def test_node_repo_dump_with_force_file(self): with tempfile.TemporaryDirectory() as tmp_dir: # First, create some other contents - file_to_replace = pathlib.Path(tmp_dir) / self.key_file1 - file_to_replace.parent.mkdir(parents=True) + file_to_replace = pathlib.Path(tmp_dir) / self.key_file2 with file_to_replace.open('w') as out_f: out_f.write('ni!') options = ['--force', str(self.folder_node.uuid), tmp_dir] - res = self.cli_runner.invoke(cmd_node.repo_dump, options, mix_stderr=False) + res = self.cli_runner.invoke(cmd_node.repo_dump, options, catch_exceptions=False) self.assertFalse(res.stdout) - # If there was no stderr, accessing the attribute raises ValueError. - with contextlib.suppress(ValueError): - self.assertFalse(res.stderr) for file_key, content in [(self.key_file1, self.content_file1), (self.key_file2, self.content_file2)]: curr_path = pathlib.Path(tmp_dir) @@ -281,10 +270,8 @@ def test_node_repo_dump_with_replace(self): out_f.write('ni!') options = [str(self.folder_node.uuid), tmp_dir] - res = self.cli_runner.invoke(cmd_node.repo_dump, options, input='r\n', mix_stderr=False) - # If there was no stderr, accessing the attribute raises ValueError. - with contextlib.suppress(ValueError): - self.assertFalse(res.stderr) + res = self.cli_runner.invoke(cmd_node.repo_dump, options, input='r\n', catch_exceptions=False) + self.assertIn('Directory', res.stdout) self.assertIn('exists', res.stdout) @@ -314,10 +301,8 @@ def test_node_repo_dump_with_replace_file(self): out_f.write('ni!') options = [str(self.folder_node.uuid), tmp_dir] - res = self.cli_runner.invoke(cmd_node.repo_dump, options, input='r\n', mix_stderr=False) - # If there was no stderr, accessing the attribute raises ValueError. - with contextlib.suppress(ValueError): - self.assertFalse(res.stderr) + res = self.cli_runner.invoke(cmd_node.repo_dump, options, input='r\n', catch_exceptions=False) + self.assertIn('File', res.stdout) self.assertIn('exists', res.stdout) @@ -347,10 +332,8 @@ def test_node_repo_dump_with_abort(self): out_f.write('ni!') options = [str(self.folder_node.uuid), tmp_dir] - res = self.cli_runner.invoke(cmd_node.repo_dump, options, input='a\n') - # If there was no stderr, accessing the attribute raises ValueError. - with contextlib.suppress(ValueError): - self.assertFalse(res.stderr) + res = self.cli_runner.invoke(cmd_node.repo_dump, options, input='a\n', catch_exceptions=False) + self.assertIn('Directory', res.stdout) self.assertIn('exists', res.stdout) self.assertIn('Aborted!', res.stdout) @@ -377,10 +360,8 @@ def test_node_repo_dump_with_skip(self): out_f.write('ni!') options = [str(self.folder_node.uuid), tmp_dir] - res = self.cli_runner.invoke(cmd_node.repo_dump, options, input='s\n') - # If there was no stderr, accessing the attribute raises ValueError. - with contextlib.suppress(ValueError): - self.assertFalse(res.stderr) + res = self.cli_runner.invoke(cmd_node.repo_dump, options, input='s\n', catch_exceptions=False) + self.assertIn('Directory', res.stdout) self.assertIn('exists', res.stdout) @@ -419,10 +400,8 @@ def test_node_repo_dump_with_merge(self): out_f.write('ni!') options = [str(self.folder_node.uuid), tmp_dir] - res = self.cli_runner.invoke(cmd_node.repo_dump, options, input='m\n') - # If there was no stderr, accessing the attribute raises ValueError. - with contextlib.suppress(ValueError): - self.assertFalse(res.stderr) + res = self.cli_runner.invoke(cmd_node.repo_dump, options, input='m\n', catch_exceptions=False) + self.assertIn('Directory', res.stdout) self.assertIn('exists', res.stdout) From 445880a9d92f2c8e31812abefd00a5b5304d84fc Mon Sep 17 00:00:00 2001 From: Dominik Gresch Date: Wed, 11 Dec 2019 18:05:47 +0100 Subject: [PATCH 11/14] Fix compatibility with Python 3.5 When passing a pathlib.Path to 'shutil', it needs to be explicitly cast to 'str'. The compatibility of 'os' and 'shutil' with pathlib objects was added only in Python 3.6. --- aiida/cmdline/commands/cmd_node.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/aiida/cmdline/commands/cmd_node.py b/aiida/cmdline/commands/cmd_node.py index b11a04c3a4..d5babff9ae 100644 --- a/aiida/cmdline/commands/cmd_node.py +++ b/aiida/cmdline/commands/cmd_node.py @@ -107,7 +107,11 @@ def _copy_tree(key, output_dir): # pylint: disable=too-many-branches elif prompt_res == 's': continue elif prompt_res == 'r': - shutil.rmtree(new_out_dir) + # Explicit 'str' cast is needed only in python <=3.5, + # because 'shutil' does not understand pathlib + # objects. This can be removed when Python3.5 + # support is dropped. + shutil.rmtree(str(new_out_dir)) new_out_dir.mkdir() else: assert prompt_res == 'm' From 023583bdfa13b8a4971a33633ad7e6c450a8ea61 Mon Sep 17 00:00:00 2001 From: Dominik Gresch Date: Thu, 12 Dec 2019 10:03:29 +0100 Subject: [PATCH 12/14] Clean up implementation of _copy_tree. --- aiida/cmdline/commands/cmd_node.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/aiida/cmdline/commands/cmd_node.py b/aiida/cmdline/commands/cmd_node.py index d5babff9ae..5f3e8ad21d 100644 --- a/aiida/cmdline/commands/cmd_node.py +++ b/aiida/cmdline/commands/cmd_node.py @@ -81,16 +81,17 @@ def repo_dump(node, output_directory, force): output_directory = pathlib.Path(output_directory) - output_directory.mkdir(exist_ok=True) - def _copy_tree(key, output_dir): # pylint: disable=too-many-branches + """ + Recursively copy the content at the ``key`` path in the given node to + the ``output_dir``. Uses prompts in case of existing files or directories. + """ if force: prompt_res = 'r' for file in node.list_objects(key=key): - if not key: - file_path = file.name - else: - file_path = key + '/' + file.name + # Not using os.path.join here, because this is the "path" + # in the AiiDA node, not an actual OS - level path. + file_key = file.name if not key else key + '/' + file.name if file.type == FileType.DIRECTORY: new_out_dir = output_dir / file.name if new_out_dir.exists(): @@ -118,10 +119,10 @@ def _copy_tree(key, output_dir): # pylint: disable=too-many-branches else: new_out_dir.mkdir() - _copy_tree(key=file_path, output_dir=new_out_dir) + _copy_tree(key=file_key, output_dir=new_out_dir) else: assert file.type == FileType.FILE - out_file_path = (output_dir / file.name) + out_file_path = output_dir / file.name if out_file_path.exists(): if not force: prompt_res = click.prompt( @@ -137,9 +138,9 @@ def _copy_tree(key, output_dir): # pylint: disable=too-many-branches else: assert prompt_res == 'r' out_file_path.unlink() - with node.open(file_path, 'rb') as in_file: + with node.open(file_key, 'rb') as in_file: with out_file_path.open('wb') as out_file: - out_file.write(in_file.read()) + shutil.copyfileobj(in_file, out_file) _copy_tree(key='', output_dir=output_directory) From 00e0d631e30a8d3b1d4c8d003236fd2673ef8420 Mon Sep 17 00:00:00 2001 From: Dominik Gresch Date: Tue, 17 Dec 2019 20:02:17 +0100 Subject: [PATCH 13/14] Simplify 'verdi node repo dump' command. Instead of prompting when a file / directory exists, we now only allow specifying an OUTPUT_DIRECTORY that does not exist, and simply abort the command if it does. --- .../tests/cmdline/commands/test_node.py | 219 ++---------------- aiida/cmdline/commands/cmd_node.py | 61 ++--- 2 files changed, 34 insertions(+), 246 deletions(-) diff --git a/aiida/backends/tests/cmdline/commands/test_node.py b/aiida/backends/tests/cmdline/commands/test_node.py index d9bd62c652..3015e48d70 100644 --- a/aiida/backends/tests/cmdline/commands/test_node.py +++ b/aiida/backends/tests/cmdline/commands/test_node.py @@ -188,232 +188,55 @@ def test_node_repo_dump(self): """Test 'verdi node repo dump' command.""" with tempfile.TemporaryDirectory() as tmp_dir: - options = [str(self.folder_node.uuid), tmp_dir] + out_path = pathlib.Path(tmp_dir) / 'out_dir' + options = [str(self.folder_node.uuid), str(out_path)] res = self.cli_runner.invoke(cmd_node.repo_dump, options, catch_exceptions=False) self.assertFalse(res.stdout) for file_key, content in [(self.key_file1, self.content_file1), (self.key_file2, self.content_file2)]: - curr_path = pathlib.Path(tmp_dir) + curr_path = out_path for key_part in file_key.split('/'): curr_path /= key_part self.assertTrue(curr_path.exists()) with curr_path.open('r') as res_file: self.assertEqual(res_file.read(), content) - def test_node_repo_dump_with_force(self): - """Test 'verdi node repo dump' command with --force option, where a directory clashes. - - This test first creates some other (clashing) content in the output - directory, but this should be replaced because of the '--force' option. - """ + def test_node_repo_dump_to_nested_folder(self): + """Test 'verdi node repo dump' command, with an output folder whose parent does not exist.""" with tempfile.TemporaryDirectory() as tmp_dir: - # First, create some other contents - sub_dir = pathlib.Path(tmp_dir) / self.key_file1.split('/')[0] - sub_dir.mkdir() - file_to_replace = (sub_dir / 'other_file') - with file_to_replace.open('w') as out_f: - out_f.write('ni!') - - options = ['--force', str(self.folder_node.uuid), tmp_dir] + out_path = pathlib.Path(tmp_dir) / 'out_dir' / 'nested' / 'path' + options = [str(self.folder_node.uuid), str(out_path)] res = self.cli_runner.invoke(cmd_node.repo_dump, options, catch_exceptions=False) self.assertFalse(res.stdout) for file_key, content in [(self.key_file1, self.content_file1), (self.key_file2, self.content_file2)]: - curr_path = pathlib.Path(tmp_dir) + curr_path = out_path for key_part in file_key.split('/'): curr_path /= key_part self.assertTrue(curr_path.exists()) with curr_path.open('r') as res_file: self.assertEqual(res_file.read(), content) - self.assertFalse(file_to_replace.exists()) - - def test_node_repo_dump_with_force_file(self): - """Test 'verdi node repo dump' command with --force option, where a file clashes. - - This test first creates a clashing file in the output - directory, but this should be replaced because of the '--force' option. - """ + def test_node_repo_existing_out_dir(self): + """Test 'verdi node repo dump' command, check that an existing output directory is not overwritten.""" with tempfile.TemporaryDirectory() as tmp_dir: - # First, create some other contents - file_to_replace = pathlib.Path(tmp_dir) / self.key_file2 - with file_to_replace.open('w') as out_f: - out_f.write('ni!') - - options = ['--force', str(self.folder_node.uuid), tmp_dir] + out_path = pathlib.Path(tmp_dir) / 'out_dir' + # Create the directory and put a file in it + out_path.mkdir() + some_file = out_path / 'file_name' + some_file_content = 'ni!' + with some_file.open('w') as file_handle: + file_handle.write(some_file_content) + options = [str(self.folder_node.uuid), str(out_path)] res = self.cli_runner.invoke(cmd_node.repo_dump, options, catch_exceptions=False) - self.assertFalse(res.stdout) - - for file_key, content in [(self.key_file1, self.content_file1), (self.key_file2, self.content_file2)]: - curr_path = pathlib.Path(tmp_dir) - for key_part in file_key.split('/'): - curr_path /= key_part - self.assertTrue(curr_path.exists()) - with curr_path.open('r') as res_file: - self.assertEqual(res_file.read(), content) - - def test_node_repo_dump_with_replace(self): - """Test 'verdi node repo dump' command with 'replace' prompt option. - - This test first creates some other (clashing) content in the output - directory, but this should be replaced because of the 'replace' option. - """ - - with tempfile.TemporaryDirectory() as tmp_dir: - # First, create some other contents - sub_dir = pathlib.Path(tmp_dir) / self.key_file1.split('/')[0] - sub_dir.mkdir() - file_to_replace = (sub_dir / 'other_file') - with file_to_replace.open('w') as out_f: - out_f.write('ni!') - - options = [str(self.folder_node.uuid), tmp_dir] - res = self.cli_runner.invoke(cmd_node.repo_dump, options, input='r\n', catch_exceptions=False) - - self.assertIn('Directory', res.stdout) - self.assertIn('exists', res.stdout) - - for file_key, content in [(self.key_file1, self.content_file1), (self.key_file2, self.content_file2)]: - curr_path = pathlib.Path(tmp_dir) - for key_part in file_key.split('/'): - curr_path /= key_part - self.assertTrue(curr_path.exists()) - with curr_path.open('r') as res_file: - self.assertEqual(res_file.read(), content) - - self.assertFalse(file_to_replace.exists()) - - def test_node_repo_dump_with_replace_file(self): - """Test 'verdi node repo dump' command with 'replace' prompt option on a file. - - This test first creates some other (clashing) content in the output - directory, but this should be replaced because of the 'replace' option. In contrast - to the other test, here the 'replace' called at the level where the files clash, - not the directories. - """ - - with tempfile.TemporaryDirectory() as tmp_dir: - # First, create some other contents - file_to_replace = pathlib.Path(tmp_dir) / self.key_file2 - with file_to_replace.open('w') as out_f: - out_f.write('ni!') - - options = [str(self.folder_node.uuid), tmp_dir] - res = self.cli_runner.invoke(cmd_node.repo_dump, options, input='r\n', catch_exceptions=False) - - self.assertIn('File', res.stdout) - self.assertIn('exists', res.stdout) - - for file_key, content in [(self.key_file1, self.content_file1), (self.key_file2, self.content_file2)]: - curr_path = pathlib.Path(tmp_dir) - for key_part in file_key.split('/'): - curr_path /= key_part - self.assertTrue(curr_path.exists()) - with curr_path.open('r') as res_file: - self.assertEqual(res_file.read(), content) - - def test_node_repo_dump_with_abort(self): - """Test 'verdi node repo dump' command with the 'abort' prompt option. - - This test first sets up another file with the same directory as where - 'file1' should be placed. Because 'abort' is selected, it should - create neither of the two files, and the directory should be left - as is. - """ - - with tempfile.TemporaryDirectory() as tmp_dir: - # First, create some other contents - sub_dir = pathlib.Path(tmp_dir) / self.key_file1.split('/')[0] - sub_dir.mkdir() - file_not_to_replace = (sub_dir / 'other_file') - with file_not_to_replace.open('w') as out_f: - out_f.write('ni!') - - options = [str(self.folder_node.uuid), tmp_dir] - res = self.cli_runner.invoke(cmd_node.repo_dump, options, input='a\n', catch_exceptions=False) - - self.assertIn('Directory', res.stdout) self.assertIn('exists', res.stdout) self.assertIn('Aborted!', res.stdout) - for file_key in [self.key_file1, self.key_file2]: - curr_path = pathlib.Path(tmp_dir) - - for key_part in file_key.split('/'): - curr_path /= key_part - if curr_path not in [sub_dir, file_not_to_replace]: - self.assertFalse(curr_path.exists()) - - self.assertTrue(file_not_to_replace.exists()) - - def test_node_repo_dump_with_skip(self): - """Test 'verdi node repo dump' command with the 'skip' prompt option.""" - - with tempfile.TemporaryDirectory() as tmp_dir: - # First, create some other contents - sub_dir = pathlib.Path(tmp_dir) / self.key_file1.split('/')[0] - sub_dir.mkdir() - file_not_to_replace = (sub_dir / 'other_file') - with file_not_to_replace.open('w') as out_f: - out_f.write('ni!') - - options = [str(self.folder_node.uuid), tmp_dir] - res = self.cli_runner.invoke(cmd_node.repo_dump, options, input='s\n', catch_exceptions=False) - - self.assertIn('Directory', res.stdout) - self.assertIn('exists', res.stdout) - - for file_key in [self.key_file1]: - curr_path = pathlib.Path(tmp_dir) - - for key_part in file_key.split('/'): - curr_path /= key_part - if curr_path not in [sub_dir, file_not_to_replace]: - self.assertFalse(curr_path.exists()) - - for file_key, content in [(self.key_file2, self.content_file2)]: - curr_path = pathlib.Path(tmp_dir) - for key_part in file_key.split('/'): - curr_path /= key_part - self.assertTrue(curr_path.exists()) - with curr_path.open('r') as res_file: - self.assertEqual(res_file.read(), content) - - self.assertTrue(file_not_to_replace.exists()) - - def test_node_repo_dump_with_merge(self): - """Test 'verdi node repo dump' command with the 'merge' prompt option. - - This test creates another file in the same (top-level) directory as - 'file1', and they should both exist in the end due to the 'merge' - prompt option. - """ - - with tempfile.TemporaryDirectory() as tmp_dir: - # First, create some other contents - sub_dir = pathlib.Path(tmp_dir) / self.key_file1.split('/')[0] - sub_dir.mkdir() - file_not_to_replace = (sub_dir / 'other_file') - with file_not_to_replace.open('w') as out_f: - out_f.write('ni!') - - options = [str(self.folder_node.uuid), tmp_dir] - res = self.cli_runner.invoke(cmd_node.repo_dump, options, input='m\n', catch_exceptions=False) - - self.assertIn('Directory', res.stdout) - self.assertIn('exists', res.stdout) - - for file_key, content in [(self.key_file1, self.content_file1), (self.key_file2, self.content_file2)]: - curr_path = pathlib.Path(tmp_dir) - for key_part in file_key.split('/'): - curr_path /= key_part - self.assertTrue(curr_path.exists()) - with curr_path.open('r') as res_file: - self.assertEqual(res_file.read(), content) - - self.assertTrue(file_not_to_replace.exists()) + # Make sure the directory content is still there + with some_file.open('r') as file_handle: + assert file_handle.read() == some_file_content def delete_temporary_file(filepath): diff --git a/aiida/cmdline/commands/cmd_node.py b/aiida/cmdline/commands/cmd_node.py index 5f3e8ad21d..53f69d25e2 100644 --- a/aiida/cmdline/commands/cmd_node.py +++ b/aiida/cmdline/commands/cmd_node.py @@ -65,79 +65,44 @@ def repo_ls(node, relative_path, color): @verdi_node_repo.command('dump') -@click.option( - '-f', '--force', help='Replace existing directories and files without prompting for confirmation.', flag_value=True -) @arguments.NODE() @click.argument( 'output_directory', - type=click.Path(exists=True, file_okay=False, writable=True), + type=click.Path(), required=True, ) @with_dbenv() -def repo_dump(node, output_directory, force): +def repo_dump(node, output_directory): """Copy the repository files of a node to an output directory.""" from aiida.orm.utils.repository import FileType output_directory = pathlib.Path(output_directory) + try: + output_directory.mkdir(parents=True, exist_ok=False) + except FileExistsError: + click.echo('Error: Invalid value for "OUTPUT_DIRECTORY": Path "{}" exists.'.format(output_directory)) + raise click.Abort + def _copy_tree(key, output_dir): # pylint: disable=too-many-branches """ Recursively copy the content at the ``key`` path in the given node to - the ``output_dir``. Uses prompts in case of existing files or directories. + the ``output_dir``. """ - if force: - prompt_res = 'r' for file in node.list_objects(key=key): # Not using os.path.join here, because this is the "path" # in the AiiDA node, not an actual OS - level path. file_key = file.name if not key else key + '/' + file.name if file.type == FileType.DIRECTORY: new_out_dir = output_dir / file.name - if new_out_dir.exists(): - if not force: - prompt_res = click.prompt( - "Directory '{}' exists. Abort [a], " - 'skip [s], merge contents [m], ' - 'or replace [r]?'.format(new_out_dir), - default='a', - type=click.Choice(['a', 's', 'm', 'r']) - ) - if prompt_res == 'a': - raise click.Abort - elif prompt_res == 's': - continue - elif prompt_res == 'r': - # Explicit 'str' cast is needed only in python <=3.5, - # because 'shutil' does not understand pathlib - # objects. This can be removed when Python3.5 - # support is dropped. - shutil.rmtree(str(new_out_dir)) - new_out_dir.mkdir() - else: - assert prompt_res == 'm' - else: - new_out_dir.mkdir() - + assert not new_out_dir.exists() + new_out_dir.mkdir() _copy_tree(key=file_key, output_dir=new_out_dir) + else: assert file.type == FileType.FILE out_file_path = output_dir / file.name - if out_file_path.exists(): - if not force: - prompt_res = click.prompt( - "File '{}' exists. Abort [a], skip [s], or replace [r]?".format(out_file_path), - default='a', - type=click.Choice(['a', 's', 'r']) - ) - - if prompt_res == 'a': - raise click.Abort - elif prompt_res == 's': - continue - else: - assert prompt_res == 'r' - out_file_path.unlink() + assert not out_file_path.exists() with node.open(file_key, 'rb') as in_file: with out_file_path.open('wb') as out_file: shutil.copyfileobj(in_file, out_file) From ef4a659fc73b3f3b6393c449b7a3f81510722ffd Mon Sep 17 00:00:00 2001 From: Dominik Gresch Date: Thu, 19 Dec 2019 12:38:13 +0100 Subject: [PATCH 14/14] Improve docstring and abort message of 'verdi node repo dump' Add a note that the output directory should not exist to the doc- string. When it does, abort with the 'echo_critical' provided by AiiDA instead of the click default. --- aiida/backends/tests/cmdline/commands/test_node.py | 2 +- aiida/cmdline/commands/cmd_node.py | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/aiida/backends/tests/cmdline/commands/test_node.py b/aiida/backends/tests/cmdline/commands/test_node.py index 3015e48d70..bee0e1c961 100644 --- a/aiida/backends/tests/cmdline/commands/test_node.py +++ b/aiida/backends/tests/cmdline/commands/test_node.py @@ -232,7 +232,7 @@ def test_node_repo_existing_out_dir(self): options = [str(self.folder_node.uuid), str(out_path)] res = self.cli_runner.invoke(cmd_node.repo_dump, options, catch_exceptions=False) self.assertIn('exists', res.stdout) - self.assertIn('Aborted!', res.stdout) + self.assertIn('Critical:', res.stdout) # Make sure the directory content is still there with some_file.open('r') as file_handle: diff --git a/aiida/cmdline/commands/cmd_node.py b/aiida/cmdline/commands/cmd_node.py index 53f69d25e2..29145421ae 100644 --- a/aiida/cmdline/commands/cmd_node.py +++ b/aiida/cmdline/commands/cmd_node.py @@ -73,7 +73,11 @@ def repo_ls(node, relative_path, color): ) @with_dbenv() def repo_dump(node, output_directory): - """Copy the repository files of a node to an output directory.""" + """Copy the repository files of a node to an output directory. + + The output directory should not exist. If it does, the command + will abort. + """ from aiida.orm.utils.repository import FileType output_directory = pathlib.Path(output_directory) @@ -81,8 +85,7 @@ def repo_dump(node, output_directory): try: output_directory.mkdir(parents=True, exist_ok=False) except FileExistsError: - click.echo('Error: Invalid value for "OUTPUT_DIRECTORY": Path "{}" exists.'.format(output_directory)) - raise click.Abort + echo.echo_critical('Invalid value for "OUTPUT_DIRECTORY": Path "{}" exists.'.format(output_directory)) def _copy_tree(key, output_dir): # pylint: disable=too-many-branches """