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.

Sign up for GitHub

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

Config: Remove use of NO_DEFAULT for Option.default #6145

Merged
merged 3 commits into from
Oct 11, 2023
Merged

Config: Remove use of NO_DEFAULT for Option.default #6145

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7e1ea7b
Tests: Print stack trace if CLI command excepts with `run_cli_command`
sphuber Oct 11, 2023
c4cea87
Config: Remove use of `NO_DEFAULT` for `Option.default`
sphuber Oct 11, 2023
60758c2
Tests: Fix failing `tests/cmdline/commands/test_setup.py`
sphuber Oct 11, 2023
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
3 changes: 1 addition & 2 deletions aiida/cmdline/commands/cmd_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,15 +77,14 @@ def _join(val):
def verdi_config_show(ctx, option):
"""Show details of an AiiDA option for the current profile."""
from aiida.manage.configuration import Config, Profile
from aiida.manage.configuration.options import NO_DEFAULT

config: Config = ctx.obj.config
profile: Profile | None = ctx.obj.profile

dct = {
'schema': option.schema,
'values': {
'default': '<NOTSET>' if option.default is NO_DEFAULT else option.default,
'default': '<NOTSET>' if option.default is None else option.default,
'global': config.options.get(option.name, '<NOTSET>'),
}
}
Expand Down
8 changes: 3 additions & 5 deletions aiida/manage/configuration/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
from aiida.common.exceptions import ConfigurationError

from . import schema as schema_module
from .options import NO_DEFAULT, Option, get_option, get_option_names, parse_option
from .options import Option, get_option, get_option_names, parse_option
from .profile import Profile

__all__ = ('Config', 'config_schema', 'ConfigValidationError')
Expand Down Expand Up @@ -407,7 +407,7 @@ def set_option(self, option_name, option_value, scope=None, override=True):

if parsed_value is not None:
value = parsed_value
elif option.default is not NO_DEFAULT:
elif option.default is not None:
value = option.default
else:
return
Expand Down Expand Up @@ -442,9 +442,7 @@ def get_option(self, option_name, scope=None, default=True):
:return: the option value or None if not set for the given scope
"""
option = get_option(option_name)

# Default value is `None` unless `default=True` and the `option.default` is not `NO_DEFAULT`
default_value = option.default if default and option.default is not NO_DEFAULT else None
default_value = option.default if default else None

if scope is not None:
value = self.get_profile(scope).get_option(option.name, default_value)
Expand Down
4 changes: 1 addition & 3 deletions aiida/manage/configuration/options.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@

__all__ = ('get_option', 'get_option_names', 'parse_option', 'Option')

NO_DEFAULT = ()


class Option:
"""Represent a configuration option schema."""
Expand All @@ -41,7 +39,7 @@ def valid_type(self) -> Any:

@property
def default(self) -> Any:
return self._schema.get('default', NO_DEFAULT)
return self._schema.get('default', None)

@property
def description(self) -> str:
Expand Down
14 changes: 9 additions & 5 deletions tests/cmdline/commands/test_setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,11 +206,15 @@ def test_setup_profile_uuid(self):

profile_name = 'profile-copy'
user_email = '[email protected]'
profile_uuid = str(uuid.uuid4)
user_first_name = 'John'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did this test work previously because these options are not required? Or because these options were not properly validated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it worked properly because the type for these options is NON_EMPTY_STRING so any string goes and () is cast to a string and then accepted 😅

user_last_name = 'Smith'
user_institution = 'ECMA'
profile_uuid = str(uuid.uuid4())
options = [
'--non-interactive', '--profile', profile_name, '--profile-uuid', profile_uuid, '--db-backend',
self.storage_backend_name, '--db-name', db_name, '--db-username', db_user, '--db-password', db_pass,
'--db-port', self.pg_test.dsn['port'], '--email', user_email
'--non-interactive', '--email', user_email, '--first-name', user_first_name, '--last-name', user_last_name,
'--institution', user_institution, '--db-name', db_name, '--db-username', db_user, '--db-password', db_pass,
'--db-port', self.pg_test.dsn['port'], '--db-backend', self.storage_backend_name, '--profile', profile_name,
'--profile-uuid', profile_uuid
]

self.cli_runner(cmd_setup.setup, options, use_subprocess=False)
Expand All @@ -219,4 +223,4 @@ def test_setup_profile_uuid(self):
assert profile_name in config.profile_names

profile = config.get_profile(profile_name)
profile.uuid = profile_uuid
assert profile.uuid == profile_uuid
5 changes: 3 additions & 2 deletions tests/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -536,8 +536,9 @@ def factory(
assert result.exception is not None, result.output
assert result.exit_code != 0, result.exit_code
else:
assert result.exception is None, result.output
assert result.exit_code == 0, result.exit_code
import traceback
assert result.exception is None, ''.join(traceback.format_exception(*result.exc_info))
assert result.exit_code == 0, (result.exit_code, result.stderr)

return result

Expand Down
Loading