From 38c4684fc770c42b81f00faa5d8156a16ca666bd Mon Sep 17 00:00:00 2001 From: Sebastiaan Huber Date: Wed, 8 Apr 2020 14:37:11 +0200 Subject: [PATCH] Add the `-v/--version` option to `verdi export migrate` (#3910) The default behavior remains the same and if not specified the export archive will be migrated to the latest version. However, with the flag any other version can be chosen, as long as it constitutes a forward migration as backward migrations are not supported. --- aiida/cmdline/commands/cmd_export.py | 24 +++++-- .../tools/importexport/migration/__init__.py | 33 +++++----- docs/source/verdi/verdi_user_guide.rst | 2 +- tests/cmdline/commands/test_export.py | 22 ++++++- .../importexport/migration/test_migration.py | 63 +++++++++---------- 5 files changed, 88 insertions(+), 56 deletions(-) diff --git a/aiida/cmdline/commands/cmd_export.py b/aiida/cmdline/commands/cmd_export.py index 4e4b8f0066..651d25ca1a 100644 --- a/aiida/cmdline/commands/cmd_export.py +++ b/aiida/cmdline/commands/cmd_export.py @@ -145,17 +145,26 @@ def create( @options.ARCHIVE_FORMAT() @options.FORCE(help='overwrite output file if it already exists') @options.SILENT() -def migrate(input_file, output_file, force, silent, archive_format): +@click.option( + '-v', + '--version', + type=click.STRING, + required=False, + metavar='VERSION', + help='Specify an exact archive version to migrate to. By default the most recent version is taken.' +) +def migrate(input_file, output_file, force, silent, archive_format, version): # pylint: disable=too-many-locals,too-many-statements,too-many-branches - """ - Migrate an old export archive file to the most recent format. - """ + """Migrate an export archive to a more recent format version.""" import tarfile import zipfile from aiida.common import json from aiida.common.folders import SandboxFolder - from aiida.tools.importexport import migration, extract_zip, extract_tar + from aiida.tools.importexport import EXPORT_VERSION, migration, extract_zip, extract_tar, ArchiveMigrationError + + if version is None: + version = EXPORT_VERSION if os.path.exists(output_file) and not force: echo.echo_critical('the output file already exists') @@ -178,7 +187,10 @@ def migrate(input_file, output_file, force, silent, archive_format): echo.echo_critical('export archive does not contain the required file {}'.format(fhandle.filename)) old_version = migration.verify_metadata_version(metadata) - new_version = migration.migrate_recursively(metadata, data, folder) + try: + new_version = migration.migrate_recursively(metadata, data, folder, version) + except ArchiveMigrationError as exception: + echo.echo_critical(exception) with open(folder.get_abs_path('data.json'), 'wb') as fhandle: json.dump(data, fhandle, indent=4) diff --git a/aiida/tools/importexport/migration/__init__.py b/aiida/tools/importexport/migration/__init__.py index e5772c1f8f..a99ee359e7 100644 --- a/aiida/tools/importexport/migration/__init__.py +++ b/aiida/tools/importexport/migration/__init__.py @@ -8,9 +8,9 @@ # For further information please visit http://www.aiida.net # ########################################################################### """Migration export files from old export versions to the newest, used by `verdi export migrate` command.""" - -from aiida.cmdline.utils import echo -from aiida.tools.importexport.common.exceptions import DanglingLinkError +from aiida.common.lang import type_check +from aiida.tools.importexport import EXPORT_VERSION +from aiida.tools.importexport.common.exceptions import DanglingLinkError, ArchiveMigrationError from .utils import verify_metadata_version from .v01_to_v02 import migrate_v1_to_v2 @@ -34,34 +34,37 @@ } -def migrate_recursively(metadata, data, folder): - """ - Recursive migration of export files from v0.1 to newest version, +def migrate_recursively(metadata, data, folder, version=EXPORT_VERSION): + """Recursive migration of export files from v0.1 to a newer version. + See specific migration functions for detailed descriptions. :param metadata: the content of an export archive metadata.json file :param data: the content of an export archive data.json file :param folder: SandboxFolder in which the archive has been unpacked (workdir) + :param version: the version to migrate to, by default the current export version """ - from aiida.tools.importexport import EXPORT_VERSION as newest_version - old_version = verify_metadata_version(metadata) + type_check(version, str) + try: - if old_version == newest_version: - echo.echo_critical('Your export file is already at the newest export version {}'.format(newest_version)) + if old_version == version: + raise ArchiveMigrationError('Your export file is already at the version {}'.format(version)) + elif old_version > version: + raise ArchiveMigrationError('Backward migrations are not supported') elif old_version in MIGRATE_FUNCTIONS: MIGRATE_FUNCTIONS[old_version](metadata, data, folder) else: - echo.echo_critical('Cannot migrate from version {}'.format(old_version)) + raise ArchiveMigrationError('Cannot migrate from version {}'.format(old_version)) except ValueError as exception: - echo.echo_critical(exception) + raise ArchiveMigrationError(exception) except DanglingLinkError: - echo.echo_critical('Export file is invalid because it contains dangling links') + raise ArchiveMigrationError('Export file is invalid because it contains dangling links') new_version = verify_metadata_version(metadata) - if new_version < newest_version: - new_version = migrate_recursively(metadata, data, folder) + if new_version < version: + new_version = migrate_recursively(metadata, data, folder, version) return new_version diff --git a/docs/source/verdi/verdi_user_guide.rst b/docs/source/verdi/verdi_user_guide.rst index 3c24ffeb1b..8265083e6d 100644 --- a/docs/source/verdi/verdi_user_guide.rst +++ b/docs/source/verdi/verdi_user_guide.rst @@ -394,7 +394,7 @@ Below is a list with all available subcommands. Commands: create Export subsets of the provenance graph to file for sharing. inspect Inspect contents of an exported archive without importing it. - migrate Migrate an old export archive file to the most recent format. + migrate Migrate an export archive to a more recent format version. .. _verdi_graph: diff --git a/tests/cmdline/commands/test_export.py b/tests/cmdline/commands/test_export.py index 4e0d5a3233..441dcfae68 100644 --- a/tests/cmdline/commands/test_export.py +++ b/tests/cmdline/commands/test_export.py @@ -19,7 +19,7 @@ from aiida.backends.testbase import AiidaTestCase from aiida.cmdline.commands import cmd_export -from aiida.tools.importexport import EXPORT_VERSION +from aiida.tools.importexport import EXPORT_VERSION, Archive from tests.utils.archives import get_archive_file @@ -160,6 +160,26 @@ def test_migrate_versions_old(self): finally: delete_temporary_file(filename_output) + def test_migrate_version_specific(self): + """Test the `-v/--version` option to migrate to a specific version instead of the latest.""" + archive = 'export_v0.1_simple.aiida' + target_version = '0.2' + + filename_input = get_archive_file(archive, filepath=self.fixture_archive) + filename_output = next(tempfile._get_candidate_names()) # pylint: disable=protected-access + + try: + options = [filename_input, filename_output, '--version', target_version] + result = self.cli_runner.invoke(cmd_export.migrate, options) + self.assertIsNone(result.exception, result.output) + self.assertTrue(os.path.isfile(filename_output)) + self.assertEqual(zipfile.ZipFile(filename_output).testzip(), None) + + with Archive(filename_output) as archive_object: + self.assertEqual(archive_object.version_format, target_version) + finally: + delete_temporary_file(filename_output) + def test_migrate_versions_recent(self): """Migrating an archive with the current version should exit with non-zero status.""" filename_input = get_archive_file(self.newest_archive, filepath=self.fixture_archive) diff --git a/tests/tools/importexport/migration/test_migration.py b/tests/tools/importexport/migration/test_migration.py index c2c45afbd2..82c08b8e1d 100644 --- a/tests/tools/importexport/migration/test_migration.py +++ b/tests/tools/importexport/migration/test_migration.py @@ -8,14 +8,12 @@ # For further information please visit http://www.aiida.net # ########################################################################### """Test export file migration from old export versions to the newest""" - import os from aiida import orm from aiida.backends.testbase import AiidaTestCase -from aiida.tools.importexport import import_data, EXPORT_VERSION as newest_version +from aiida.tools.importexport import import_data, ArchiveMigrationError, Archive, EXPORT_VERSION as newest_version from aiida.tools.importexport.migration import migrate_recursively, verify_metadata_version -from aiida.common.utils import Capturing from tests.utils.archives import get_archive_file, get_json_files, migrate_archive from tests.utils.configuration import with_temp_dir @@ -102,6 +100,28 @@ def test_migrate_recursively(self): verify_metadata_version(metadata, version=newest_version) self.assertEqual(new_version, newest_version) + def test_migrate_recursively_specific_version(self): + """Test the `version` argument of the `migrate_recursively` function.""" + filepath_archive = get_archive_file('export_v0.3_simple.aiida', **self.core_archive) + + with Archive(filepath_archive) as archive: + + # Incorrect type + with self.assertRaises(TypeError): + migrate_recursively(archive.meta_data, archive.data, None, version=0.2) + + # Backward migrations are not supported + with self.assertRaises(ArchiveMigrationError): + migrate_recursively(archive.meta_data, archive.data, None, version='0.2') + + # Same version will also raise + with self.assertRaises(ArchiveMigrationError): + migrate_recursively(archive.meta_data, archive.data, None, version='0.3') + + migrated_version = '0.5' + version = migrate_recursively(archive.meta_data, archive.data, None, version=migrated_version) + self.assertEqual(version, migrated_version) + @with_temp_dir def test_no_node_export(self, temp_dir): """Test migration of export file that has no Nodes""" @@ -138,7 +158,6 @@ def test_wrong_versions(self): """Test correct errors are raised if export files have wrong version numbers""" from aiida.tools.importexport.migration import MIGRATE_FUNCTIONS - # Initialization wrong_versions = ['0.0', '0.1.0', '0.99'] old_versions = list(MIGRATE_FUNCTIONS.keys()) legal_versions = old_versions + [newest_version] @@ -147,7 +166,6 @@ def test_wrong_versions(self): metadata = {'export_version': version} wrong_version_metadatas.append(metadata) - # Checks # Make sure the "wrong_versions" are wrong for version in wrong_versions: self.assertNotIn( @@ -156,18 +174,11 @@ def test_wrong_versions(self): msg="'{}' was not expected to be a legal version, legal version: {}".format(version, legal_versions) ) - # Make sure migrate_recursively throws a critical message and raises SystemExit + # Make sure migrate_recursively throws an ArchiveMigrationError for metadata in wrong_version_metadatas: - with self.assertRaises(SystemExit) as exception: - with Capturing(capture_stderr=True): - new_version = migrate_recursively(metadata, {}, None) - - self.assertIn( - 'Critical: Cannot migrate from version {}'.format(metadata['export_version']), - exception.exception, - msg="Expected a critical statement for the wrong export version '{}', " - 'instead got {}'.format(metadata['export_version'], exception.exception) - ) + with self.assertRaises(ArchiveMigrationError): + new_version = migrate_recursively(metadata, {}, None) + self.assertIsNone( new_version, msg='migrate_recursively should not return anything, ' @@ -175,26 +186,12 @@ def test_wrong_versions(self): ) def test_migrate_newest_version(self): - """ - Test critical message and SystemExit is raised, when an export file with the newest export version is migrated - """ - # Initialization + """Test that an exception is raised when an export file with the newest export version is migrated.""" metadata = {'export_version': newest_version} - # Check - with self.assertRaises(SystemExit) as exception: + with self.assertRaises(ArchiveMigrationError): + new_version = migrate_recursively(metadata, {}, None) - with Capturing(capture_stderr=True): - new_version = migrate_recursively(metadata, {}, None) - - self.assertIn( - 'Critical: Your export file is already at the newest export version {}'.format( - metadata['export_version'] - ), - exception.exception, - msg="Expected a critical statement that the export version '{}' is the newest export version '{}', " - 'instead got {}'.format(metadata['export_version'], newest_version, exception.exception) - ) self.assertIsNone( new_version, msg='migrate_recursively should not return anything, '