Skip to content

Commit

Permalink
Manager.load_profile: do not load default profile if other already …
Browse files Browse the repository at this point in the history
…loaded

The `Manager.load_profile` method was loading the default profile if no
explicit profile name was specified, even if another profile was already
loaded. This is violating the note in the docstring that if a profile is
already loaded, not specifying an explicit profile to be loaded should
be a no-op.

This bug was manifesting itself in `verdi` where the `--profile` option
was effectively ignored if a profile other than the default was
specified for a command that uses the `load_dbenv` decorator. This is
because the `ProfileParamType` would correctly load the profile
specified by the option, but then the `load_dbenv` decorator would call
`aiida.cmdline.utils.decorators.load_backend_if_not_loaded` which would
call `Manager.load_profile` without specifying a profile, and so the
default profile would be loaded.
  • Loading branch information
sphuber committed Feb 23, 2022
1 parent 19b945b commit 7b50516
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 1 deletion.
6 changes: 5 additions & 1 deletion aiida/manage/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def get_profile(self) -> Optional['Profile']:
def load_profile(self, profile: Union[None, str, 'Profile'] = None, allow_switch=False) -> 'Profile':
"""Load a global profile, unloading any previously loaded profile.
.. note:: if a profile is already loaded and no explicit profile is specified, nothing will be done
.. note:: If a profile is already loaded and no explicit profile is specified, nothing will be done.
:param profile: the name of the profile to load, by default will use the one marked as default in the config
:param allow_switch: if True, will allow switching to a different profile when storage is already loaded
Expand All @@ -105,6 +105,10 @@ def load_profile(self, profile: Union[None, str, 'Profile'] = None, allow_switch
from aiida.common.log import configure_logging
from aiida.manage.configuration.profile import Profile

# If a profile is already loaded and no explicit profile is specified, we do nothing
if profile is None and self._profile:
return self._profile

if profile is None or isinstance(profile, str):
profile = self.get_config().get_profile(profile)
elif not isinstance(profile, Profile):
Expand Down
69 changes: 69 additions & 0 deletions tests/cmdline/utils/test_decorators.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
# -*- coding: utf-8 -*-
###########################################################################
# Copyright (c), The AiiDA team. All rights reserved. #
# This file is part of the AiiDA code. #
# #
# The code is hosted on GitHub at https://github.com/aiidateam/aiida-core #
# For further information on the license, see the LICENSE.txt file #
# For further information please visit http://www.aiida.net #
###########################################################################
# pylint: disable=redefined-outer-name
"""Tests for the :mod:`aiida.cmdline.utils.decorators` module."""
import pytest

from aiida.cmdline.utils.decorators import load_backend_if_not_loaded
from aiida.common.exceptions import InvalidOperation
from aiida.manage import get_manager


@pytest.fixture
def config(empty_config, profile_factory):
"""Return an isolated configuration with two profiles configured and the first set as the default."""
config = empty_config
profile_one = profile_factory(name='profile-one')
profile_two = profile_factory(name='profile-two')
config.add_profile(profile_one)
config.add_profile(profile_two)
config.set_default_profile(profile_one.name)
yield config


@pytest.fixture
def manager(monkeypatch):
"""Return a ``Manager`` instance with the ``get_profile_storage`` method mocked."""
manager = get_manager()

class StorageBackend:
"""Mock version of :class:`aiida.orm.implementation.storage_backend.StorageBackend`."""

def close(self):
pass

def get_profile_storage(self):
"""Set a mock version of the storage backend."""
self._profile_storage = StorageBackend() # pylint: disable=protected-access

monkeypatch.setattr(manager.__class__, 'get_profile_storage', get_profile_storage)
yield manager


def test_load_backend_if_not_loaded(config, manager):
"""Test the :meth:`aiida.cmdline.utils.decorators.load_backend_if_not_loaded` if no profile is loaded."""
assert manager.get_profile() is None

load_backend_if_not_loaded()
assert manager.get_profile().name == config.default_profile_name

with pytest.raises(InvalidOperation, match=r'cannot switch to profile .* allow_switch is False'):
manager.load_profile('profile-two')


def test_load_backend_if_not_loaded_with_loaded_profile(config, manager):
"""Test the :meth:`aiida.cmdline.utils.decorators.load_backend_if_not_loaded` if a profile is already loaded."""
manager.load_profile('profile-two')
assert manager.get_profile().name == 'profile-two'
assert config.default_profile_name != 'profile-two'

# Calling the method again should keep the currently loaded profile, and not switch to the default profile
load_backend_if_not_loaded()
assert manager.get_profile().name == 'profile-two'

0 comments on commit 7b50516

Please sign in to comment.