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

Profile: formalize what constitutes a test profile #5392

Merged
merged 2 commits into from
Feb 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/config/profile.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,4 @@ broker_host: 127.0.0.1
broker_port: 5672
broker_virtual_host: ''
repository: /tmp/test_repository_test_aiida/
test_profile: true
8 changes: 6 additions & 2 deletions aiida/cmdline/commands/cmd_setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,12 @@
@options_setup.SETUP_BROKER_PORT()
@options_setup.SETUP_BROKER_VIRTUAL_HOST()
@options_setup.SETUP_REPOSITORY_URI()
@options_setup.SETUP_TEST_PROFILE()
@options.CONFIG_FILE()
def setup(
non_interactive, profile: Profile, email, first_name, last_name, institution, db_engine, db_backend, db_host,
db_port, db_name, db_username, db_password, broker_protocol, broker_username, broker_password, broker_host,
broker_port, broker_virtual_host, repository
broker_port, broker_virtual_host, repository, test_profile
):
"""Setup a new profile.

Expand Down Expand Up @@ -74,6 +75,7 @@ def setup(
'broker_virtual_host': broker_virtual_host,
}
)
profile.is_test_profile = test_profile

config = get_config()

Expand Down Expand Up @@ -142,12 +144,13 @@ def setup(
@options_setup.QUICKSETUP_BROKER_PORT()
@options_setup.QUICKSETUP_BROKER_VIRTUAL_HOST()
@options_setup.QUICKSETUP_REPOSITORY_URI()
@options_setup.QUICKSETUP_TEST_PROFILE()
@options.CONFIG_FILE()
@click.pass_context
def quicksetup(
ctx, non_interactive, profile, email, first_name, last_name, institution, db_engine, db_backend, db_host, db_port,
db_name, db_username, db_password, su_db_name, su_db_username, su_db_password, broker_protocol, broker_username,
broker_password, broker_host, broker_port, broker_virtual_host, repository
broker_password, broker_host, broker_port, broker_virtual_host, repository, test_profile
):
"""Setup a new profile in a fully automated fashion."""
# pylint: disable=too-many-arguments,too-many-locals
Expand Down Expand Up @@ -202,5 +205,6 @@ def quicksetup(
'broker_port': broker_port,
'broker_virtual_host': broker_virtual_host,
'repository': repository,
'test_profile': test_profile,
}
ctx.invoke(setup, **setup_parameters)
6 changes: 6 additions & 0 deletions aiida/cmdline/params/options/commands/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -360,3 +360,9 @@ def get_quicksetup_password(ctx, param, value): # pylint: disable=unused-argume
contextual_default=get_repository_uri_default,
cls=options.interactive.InteractiveOption
)

SETUP_TEST_PROFILE = options.OverridableOption(
'--test-profile', is_flag=True, help='Designate the profile to be used for running the test suite only.'
)

QUICKSETUP_TEST_PROFILE = SETUP_TEST_PROFILE.clone()
2 changes: 1 addition & 1 deletion aiida/manage/configuration/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@

__all__ = ('Config', 'config_schema', 'ConfigValidationError')

SCHEMA_FILE = 'config-v7.schema.json'
SCHEMA_FILE = 'config-v8.schema.json'


@lru_cache(1)
Expand Down
52 changes: 50 additions & 2 deletions aiida/manage/configuration/migrations/migrations.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@
# When the configuration file format is changed in a backwards-incompatible way, the oldest compatible version should
# be set to the new current version.

CURRENT_CONFIG_VERSION = 7
OLDEST_COMPATIBLE_CONFIG_VERSION = 7
CURRENT_CONFIG_VERSION = 8
OLDEST_COMPATIBLE_CONFIG_VERSION = 8

CONFIG_LOGGER = AIIDA_LOGGER.getChild('config')

Expand Down Expand Up @@ -296,6 +296,53 @@ def downgrade(self, config: ConfigType) -> None:
CONFIG_LOGGER.warning(f'profile {profile_name!r} had no expected "storage._v6_backend" key')


class AddTestProfileKey(SingleMigration):
"""Add the ``test_profile`` key."""
down_revision = 7
down_compatible = 7
up_revision = 8
up_compatible = 8

def upgrade(self, config: ConfigType) -> None:
for profile_name, profile in config.get('profiles', {}).items():
profile['test_profile'] = profile_name.startswith('test_')

def downgrade(self, config: ConfigType) -> None:
profiles = config.get('profiles', {})
profile_names = list(profiles.keys())

# Iterate over the fixed list of the profile names, since we are mutating the profiles dictionary.
for profile_name in profile_names:
Copy link
Member

Choose a reason for hiding this comment

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

Nice! I see you are still pushing changes, so ping me when you want the final review.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am done. Everything is addressed and commits are rebased


profile = profiles.pop(profile_name)
profile_name_new = None
test_profile = profile.pop('test_profile', False) # If absent, assume it is not a test profile

if test_profile and not profile_name.startswith('test_'):
profile_name_new = f'test_{profile_name}'
CONFIG_LOGGER.warning(
f'profile `{profile_name}` is a test profile but does not start with the required `test_` prefix.'
)

if not test_profile and profile_name.startswith('test_'):
profile_name_new = profile_name[5:]
CONFIG_LOGGER.warning(
f'profile `{profile_name}` is not a test profile but starts with the `test_` prefix.'
)

if profile_name_new is not None:

if profile_name_new in profile_names:
raise exceptions.ConfigurationError(
f'cannot change `{profile_name}` to `{profile_name_new}` because it already exists.'
)

CONFIG_LOGGER.warning(f'changing profile name from `{profile_name}` to `{profile_name_new}`.')
profile_name = profile_name_new

profiles[profile_name] = profile


MIGRATIONS = (
Initial,
AddProfileUuid,
Expand All @@ -304,6 +351,7 @@ def downgrade(self, config: ConfigType) -> None:
SimplifyOptions,
AbstractStorageAndProcess,
MergeStorageBackendTypes,
AddTestProfileKey,
)


Expand Down
14 changes: 12 additions & 2 deletions aiida/manage/configuration/profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ class Profile: # pylint: disable=too-many-public-methods
KEY_PROCESS_BACKEND = 'backend'
KEY_PROCESS_CONFIG = 'config'
KEY_OPTIONS = 'options'
KEY_TEST_PROFILE = 'test_profile'

# keys that are expected to be in the parsed configuration
REQUIRED_KEYS = (
Expand Down Expand Up @@ -199,8 +200,17 @@ def is_test_profile(self) -> bool:

:return: boolean, True if test profile, False otherwise
"""
# Currently, whether a profile is a test profile is solely determined by its name starting with 'test_'
return self.name.startswith('test_')
# Check explicitly for ``True`` for safety. If an invalid value is defined, we default to treating it as not
# a test profile as that can unintentionally clear the database.
return self._attributes.get(self.KEY_TEST_PROFILE, False) is True

@is_test_profile.setter
def is_test_profile(self, value: bool) -> None:
"""Set whether the profile is a test profile.

:param value: boolean indicating whether this profile is a test profile.
"""
self._attributes[self.KEY_TEST_PROFILE] = value

@property
def repository_path(self) -> pathlib.Path:
Expand Down
Loading