Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

👌 IMPROVE: Configuration migrations #5319

Merged
merged 14 commits into from
Jan 21, 2022
32 changes: 32 additions & 0 deletions aiida/cmdline/commands/cmd_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,17 @@
# For further information please visit http://www.aiida.net #
###########################################################################
"""`verdi config` command."""
import json
from pathlib import Path
import textwrap

import click

from aiida.cmdline.commands.cmd_verdi import verdi
from aiida.cmdline.params import arguments
from aiida.cmdline.utils import echo
from aiida.manage.configuration import downgrade_config, get_config_path
from aiida.manage.configuration.settings import DEFAULT_CONFIG_INDENT_SIZE


@verdi.group('config')
Expand Down Expand Up @@ -190,3 +194,31 @@ def verdi_config_caching(disabled):
echo.echo(identifier)
elif disabled:
echo.echo(identifier)


@verdi_config.command('downgrade')
@click.argument('version', type=int)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make this a type=click.Choice and get the available numbers dynamically from the available migrations? That would make it a lot better from a UI perspective.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@click.option(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct me if I am wrong, but I don't think you answered my question yet why we need the option to specify an arbitrary input and output file. Since this is user facing it should simply always operate on the currently configured config file. I would suggest to remove this additional complexity since I don't think it is needed for now and you don't have tests for it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

'-p',
'--path',
default=None,
type=click.Path(exists=True, path_type=Path),
help='Path to the config file (default to current configuration).'
)
@click.option(
'-o',
'--output',
default=None,
type=click.Path(exists=True, path_type=Path),
help='Path to write to (default to input path).'
)
def verdi_config_downgrade(version, path, output):
"""Print a configuration, downgraded to a specific version."""
path = path or Path(get_config_path())
echo.echo_report(f'Downgrading configuration to v{version}: {path}')
config = json.loads(path.read_text(encoding='utf8'))
downgrade_config(config, version)
output = Path(output or path)
output.parent.mkdir(parents=True, exist_ok=True)
output.write_text(json.dumps(config, indent=DEFAULT_CONFIG_INDENT_SIZE), encoding='utf8')
echo.echo_success(f'Downgraded configuration written to: {output}')
2 changes: 2 additions & 0 deletions aiida/manage/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
'config_needs_migrating',
'config_schema',
'disable_caching',
'downgrade_config',
'enable_caching',
'get_current_version',
'get_manager',
Expand All @@ -58,6 +59,7 @@
'get_use_cache',
'parse_option',
'reset_manager',
'upgrade_config',
'write_database_integrity_violation',
)

Expand Down
2 changes: 2 additions & 0 deletions aiida/manage/configuration/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,12 @@
'check_and_migrate_config',
'config_needs_migrating',
'config_schema',
'downgrade_config',
'get_current_version',
'get_option',
'get_option_names',
'parse_option',
'upgrade_config',
)

# yapf: enable
Expand Down
2 changes: 1 addition & 1 deletion aiida/manage/configuration/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ def from_file(cls, filepath):
migrated = False

# If the configuration file needs to be migrated first create a specific backup so it can easily be reverted
if config_needs_migrating(config):
if config_needs_migrating(config, filepath):
migrated = True
echo.echo_warning(f'current configuration file `{filepath}` is outdated and will be migrated')
filepath_backup = cls._backup(filepath)
Expand Down
3 changes: 2 additions & 1 deletion aiida/manage/configuration/migrations/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,15 @@
# pylint: disable=wildcard-import

from .migrations import *
from .utils import *

__all__ = (
'CURRENT_CONFIG_VERSION',
'OLDEST_COMPATIBLE_CONFIG_VERSION',
'check_and_migrate_config',
'config_needs_migrating',
'downgrade_config',
'get_current_version',
'upgrade_config',
)

# yapf: enable
Loading