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

Use only one global var for marking config folder tree #6610

Merged
merged 35 commits into from
Nov 27, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
fdff314
Use only one global var for marking config folder tree
unkcpz Nov 14, 2024
f7398af
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Nov 14, 2024
ec3ebec
part
unkcpz Nov 14, 2024
2663d7e
with global fix
unkcpz Nov 14, 2024
418cc2a
Rename global AIIDA_CONFIG_FOLDER as lowercase var
unkcpz Nov 15, 2024
d7f3684
Remove use of ACCESS_CONTROL_DIR
unkcpz Nov 15, 2024
959a9ad
Remove use of DAEMON_DIR
unkcpz Nov 15, 2024
717cf06
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Nov 15, 2024
5ed4950
Unbounded warning fix
unkcpz Nov 15, 2024
1b26044
src/aiida/manage/configuration/profile.py out of mypy exclude list
unkcpz Nov 15, 2024
c4adbec
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Nov 15, 2024
8795935
Update src/aiida/manage/configuration/settings.py
unkcpz Nov 15, 2024
ad1086f
fj
unkcpz Nov 15, 2024
95e305c
global object pattern applied
unkcpz Nov 15, 2024
dd7f04f
Update src/aiida/manage/configuration/__init__.py
unkcpz Nov 19, 2024
2ce9080
hehe, this looks like real Singleton
unkcpz Nov 19, 2024
c705267
Merge branch 'main' into proper-global-var-singleton
unkcpz Nov 19, 2024
8408ef5
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Nov 19, 2024
aa6b44d
Explicitly load and set configuration folder in the main module level
unkcpz Nov 20, 2024
bdfedca
rbsad
unkcpz Nov 22, 2024
a9980a2
Resolver to resolve, AiiDAConfigDir is singleton
unkcpz Nov 22, 2024
627d0da
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Nov 22, 2024
6cef555
rsb
unkcpz Nov 22, 2024
b5cb0d7
rdh
unkcpz Nov 22, 2024
6ec11a4
Move filepaths to config
unkcpz Nov 22, 2024
88fe04d
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Nov 22, 2024
0d87b9b
f-pre-commit
unkcpz Nov 22, 2024
e8919dc
Merge branch 'main' into proper-global-var-singleton
unkcpz Nov 25, 2024
aec9321
Merge branch 'main' into proper-global-var-singleton
unkcpz Nov 26, 2024
9fd3dae
Rename the set/get method of AiiDAConfigDir to just set and get
unkcpz Nov 26, 2024
35b7ae2
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Nov 26, 2024
341bab5
Revert changes for type checkings
unkcpz Nov 26, 2024
b4e40de
Merge branch 'main' into proper-global-var-singleton
unkcpz Nov 26, 2024
b540813
rseb
unkcpz Nov 27, 2024
6e26eaf
rda
unkcpz Nov 27, 2024
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: 0 additions & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,6 @@ repos:
src/aiida/engine/processes/ports.py|
src/aiida/manage/configuration/__init__.py|
src/aiida/manage/configuration/config.py|
src/aiida/manage/configuration/profile.py|
unkcpz marked this conversation as resolved.
Show resolved Hide resolved
src/aiida/manage/external/rmq/launcher.py|
src/aiida/manage/tests/main.py|
src/aiida/manage/tests/pytest_fixtures.py|
Expand Down
4 changes: 2 additions & 2 deletions src/aiida/cmdline/commands/cmd_presto.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ def detect_postgres_config(
"""
import secrets

from aiida.manage.configuration.settings import AIIDA_CONFIG_FOLDER
from aiida.manage.configuration.settings import glb_aiida_config_folder
from aiida.manage.external.postgres import Postgres

dbinfo = {
Expand Down Expand Up @@ -98,7 +98,7 @@ def detect_postgres_config(
'database_name': database_name,
'database_username': database_username,
'database_password': database_password,
'repository_uri': f'file://{AIIDA_CONFIG_FOLDER / "repository" / profile_name}',
'repository_uri': f'file://{glb_aiida_config_folder / "repository" / profile_name}',
}


Expand Down
4 changes: 2 additions & 2 deletions src/aiida/cmdline/commands/cmd_profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,9 @@
# This can happen for a fresh install and the `verdi setup` has not yet been run. In this case it is still nice
# to be able to see the configuration directory, for instance for those who have set `AIIDA_PATH`. This way
# they can at least verify that it is correctly set.
from aiida.manage.configuration.settings import AIIDA_CONFIG_FOLDER
from aiida.manage.configuration.settings import glb_aiida_config_folder

Check warning on line 172 in src/aiida/cmdline/commands/cmd_profile.py

View check run for this annotation

Codecov / codecov/patch

src/aiida/cmdline/commands/cmd_profile.py#L172

Added line #L172 was not covered by tests

echo.echo_report(f'configuration folder: {AIIDA_CONFIG_FOLDER}')
echo.echo_report(f'configuration folder: {glb_aiida_config_folder}')

Check warning on line 174 in src/aiida/cmdline/commands/cmd_profile.py

View check run for this annotation

Codecov / codecov/patch

src/aiida/cmdline/commands/cmd_profile.py#L174

Added line #L174 was not covered by tests
echo.echo_critical(str(exception))
else:
echo.echo_report(f'configuration folder: {config.dirpath}')
Expand Down
4 changes: 2 additions & 2 deletions src/aiida/cmdline/commands/cmd_status.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,13 @@ def verdi_status(print_traceback, no_rmq):
from aiida.common.docs import URL_NO_BROKER
from aiida.common.exceptions import ConfigurationError
from aiida.engine.daemon.client import DaemonException, DaemonNotRunningException
from aiida.manage.configuration.settings import AIIDA_CONFIG_FOLDER
from aiida.manage.configuration.settings import glb_aiida_config_folder
from aiida.manage.manager import get_manager

exit_code = ExitCode.SUCCESS

print_status(ServiceStatus.UP, 'version', f'AiiDA v{__version__}')
print_status(ServiceStatus.UP, 'config', AIIDA_CONFIG_FOLDER)
print_status(ServiceStatus.UP, 'config', glb_aiida_config_folder)

manager = get_manager()

Expand Down
4 changes: 2 additions & 2 deletions src/aiida/cmdline/params/options/commands/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,11 @@ def get_repository_uri_default(ctx):
"""
import os

from aiida.manage.configuration.settings import AIIDA_CONFIG_FOLDER
from aiida.manage.configuration.settings import glb_aiida_config_folder

validate_profile_parameter(ctx)

return os.path.join(AIIDA_CONFIG_FOLDER, 'repository', ctx.params['profile'].name)
return os.path.join(glb_aiida_config_folder, 'repository', ctx.params['profile'].name)


def get_quicksetup_repository_uri(ctx, param, value):
Expand Down
4 changes: 2 additions & 2 deletions src/aiida/manage/configuration/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,9 @@

def get_config_path():
"""Returns path to .aiida configuration directory."""
danielhollas marked this conversation as resolved.
Show resolved Hide resolved
from .settings import AIIDA_CONFIG_FOLDER, DEFAULT_CONFIG_FILE_NAME
from .settings import DEFAULT_CONFIG_FILE_NAME, glb_aiida_config_folder

return os.path.join(AIIDA_CONFIG_FOLDER, DEFAULT_CONFIG_FILE_NAME)
return os.path.join(glb_aiida_config_folder, DEFAULT_CONFIG_FILE_NAME)


def load_config(create=False) -> 'Config':
Expand Down
77 changes: 44 additions & 33 deletions src/aiida/manage/configuration/profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,15 @@

from __future__ import annotations

import collections
import os
import pathlib
from collections import abc
from copy import deepcopy
from typing import TYPE_CHECKING, Any, Dict, Mapping, Optional, Type
from typing import TYPE_CHECKING, Any, cast

from aiida.common import exceptions
from aiida.common.lang import type_check
from aiida.manage.configuration.settings import AiiDAConfigPathResolver

from .options import parse_option

Expand All @@ -29,41 +31,44 @@
class Profile:
"""Class that models a profile as it is stored in the configuration file of an AiiDA instance."""

KEY_UUID = 'PROFILE_UUID'
KEY_DEFAULT_USER_EMAIL = 'default_user_email'
KEY_STORAGE = 'storage'
KEY_PROCESS = 'process_control'
KEY_STORAGE_BACKEND = 'backend'
KEY_STORAGE_CONFIG = 'config'
KEY_PROCESS_BACKEND = 'backend'
KEY_PROCESS_CONFIG = 'config'
KEY_OPTIONS = 'options'
KEY_TEST_PROFILE = 'test_profile'
KEY_UUID: str = 'PROFILE_UUID'
KEY_DEFAULT_USER_EMAIL: str = 'default_user_email'
KEY_STORAGE: str = 'storage'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are all of these type annotations necessary? I would expect these types would be inferred.

Does mypy complain when you remove them?

Copy link
Member Author

Choose a reason for hiding this comment

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

mypy not complain, again, it is a suggestion from pyright. For the class attributes, if the class is not annotate with @Final, better to add type to avoid being wrongly override.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Too keep the diff small, I would prefer to not do unrelated changes in this PR, since it should be reviewed carefully given that it touches core functionality.

KEY_PROCESS: str = 'process_control'
KEY_STORAGE_BACKEND: str = 'backend'
KEY_STORAGE_CONFIG: str = 'config'
KEY_PROCESS_BACKEND: str = 'backend'
KEY_PROCESS_CONFIG: str = 'config'
KEY_OPTIONS: str = 'options'
KEY_TEST_PROFILE: str = 'test_profile'

# keys that are expected to be in the parsed configuration
REQUIRED_KEYS = (
REQUIRED_KEYS: tuple[str, str] = (
KEY_STORAGE,
KEY_PROCESS,
)

def __init__(self, name: str, config: Mapping[str, Any], validate=True):
def __init__(
self, name: str, config: abc.Mapping[str, Any], config_folder: pathlib.Path | None = None, validate: bool = True
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am confused. You added a config_folder parameter here, but I don't see where you changed the places where the Profile objects are created?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this can be removed. I put here just to show the profile class is independent of the default config folder. In principle, it should be possible to create a profile from the other config folder and mapping to the correct log/pid files.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good to know, but unless we actually use it somewhere I would remove this for now.

):
"""Load a profile with the profile configuration."""
if not isinstance(config, collections.abc.Mapping):
raise TypeError(f'config should be a mapping but is {type(config)}')
_ = type_check(config, abc.Mapping)
unkcpz marked this conversation as resolved.
Show resolved Hide resolved
if validate and not set(config.keys()).issuperset(self.REQUIRED_KEYS):
raise exceptions.ConfigurationError(
f'profile {name!r} configuration does not contain all required keys: {self.REQUIRED_KEYS}'
)

self._name = name
self._attributes: Dict[str, Any] = deepcopy(config)
self._name: str = name
self._attributes: dict[str, Any] = cast(dict[str, Any], deepcopy(config))
danielhollas marked this conversation as resolved.
Show resolved Hide resolved

# Create a default UUID if not specified
if self._attributes.get(self.KEY_UUID, None) is None:
from uuid import uuid4

self._attributes[self.KEY_UUID] = uuid4().hex

self._config_path_resolver: AiiDAConfigPathResolver = AiiDAConfigPathResolver(config_folder)
unkcpz marked this conversation as resolved.
Show resolved Hide resolved

def __repr__(self) -> str:
return f'Profile<uuid={self.uuid!r} name={self.name!r}>'

Expand All @@ -84,12 +89,17 @@ def uuid(self, value: str) -> None:
self._attributes[self.KEY_UUID] = value

@property
def default_user_email(self) -> Optional[str]:
def config_path_resolver(self) -> AiiDAConfigPathResolver:
"""The config_path_resolver property."""
return self._config_path_resolver

@property
def default_user_email(self) -> str | None:
"""Return the default user email."""
return self._attributes.get(self.KEY_DEFAULT_USER_EMAIL, None)

@default_user_email.setter
def default_user_email(self, value: Optional[str]) -> None:
def default_user_email(self, value: str | None) -> None:
unkcpz marked this conversation as resolved.
Show resolved Hide resolved
"""Set the default user email."""
self._attributes[self.KEY_DEFAULT_USER_EMAIL] = value

Expand All @@ -99,11 +109,11 @@ def storage_backend(self) -> str:
return self._attributes[self.KEY_STORAGE][self.KEY_STORAGE_BACKEND]

@property
def storage_config(self) -> Dict[str, Any]:
def storage_config(self) -> dict[str, Any]:
"""Return the configuration required by the storage backend."""
return self._attributes[self.KEY_STORAGE][self.KEY_STORAGE_CONFIG]

def set_storage(self, name: str, config: Dict[str, Any]) -> None:
def set_storage(self, name: str, config: dict[str, Any]) -> None:
"""Set the storage backend and its configuration.

:param name: the name of the storage backend
Expand All @@ -114,7 +124,7 @@ def set_storage(self, name: str, config: Dict[str, Any]) -> None:
self._attributes[self.KEY_STORAGE][self.KEY_STORAGE_CONFIG] = config

@property
def storage_cls(self) -> Type['StorageBackend']:
def storage_cls(self) -> type['StorageBackend']:
"""Return the storage backend class for this profile."""
from aiida.plugins import StorageFactory

Expand All @@ -126,11 +136,11 @@ def process_control_backend(self) -> str | None:
return self._attributes[self.KEY_PROCESS][self.KEY_PROCESS_BACKEND]

@property
def process_control_config(self) -> Dict[str, Any]:
def process_control_config(self) -> dict[str, Any]:
"""Return the configuration required by the process control backend."""
return self._attributes[self.KEY_PROCESS][self.KEY_PROCESS_CONFIG] or {}

def set_process_controller(self, name: str, config: Dict[str, Any]) -> None:
def set_process_controller(self, name: str, config: dict[str, Any]) -> None:
"""Set the process control backend and its configuration.

:param name: the name of the process backend
Expand Down Expand Up @@ -175,7 +185,7 @@ def name(self):
return self._name

@property
def dictionary(self) -> Dict[str, Any]:
def dictionary(self) -> dict[str, Any]:
"""Return the profile attributes as a dictionary with keys as it is stored in the config

:return: the profile configuration dictionary
Expand Down Expand Up @@ -235,22 +245,23 @@ def filepaths(self):

:return: a dictionary of filepaths
"""
from .settings import DAEMON_DIR, DAEMON_LOG_DIR
daemon_dir = self._config_path_resolver.daemon_dir
daemon_log_dir = self._config_path_resolver.daemon_log_dir

return {
'circus': {
'log': str(DAEMON_LOG_DIR / f'circus-{self.name}.log'),
'pid': str(DAEMON_DIR / f'circus-{self.name}.pid'),
'port': str(DAEMON_DIR / f'circus-{self.name}.port'),
'log': str(daemon_log_dir / f'circus-{self.name}.log'),
'pid': str(daemon_dir / f'circus-{self.name}.pid'),
'port': str(daemon_dir / f'circus-{self.name}.port'),
'socket': {
'file': str(DAEMON_DIR / f'circus-{self.name}.sockets'),
'file': str(daemon_dir / f'circus-{self.name}.sockets'),
'controller': 'circus.c.sock',
'pubsub': 'circus.p.sock',
'stats': 'circus.s.sock',
},
},
'daemon': {
'log': str(DAEMON_LOG_DIR / f'aiida-{self.name}.log'),
'pid': str(DAEMON_DIR / f'aiida-{self.name}.pid'),
'log': str(daemon_log_dir / f'aiida-{self.name}.log'),
'pid': str(daemon_dir / f'aiida-{self.name}.pid'),
},
}
76 changes: 48 additions & 28 deletions src/aiida/manage/configuration/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import os
import pathlib
import warnings
from typing import final

DEFAULT_UMASK = 0o0077
DEFAULT_AIIDA_PATH_VARIABLE = 'AIIDA_PATH'
Expand All @@ -26,45 +27,69 @@
DEFAULT_ACCESS_CONTROL_DIR_NAME = 'access'

# Assign defaults which may be overriden in set_configuration_directory() below
AIIDA_CONFIG_FOLDER: pathlib.Path = pathlib.Path(DEFAULT_AIIDA_PATH).expanduser() / DEFAULT_CONFIG_DIR_NAME
DAEMON_DIR: pathlib.Path = AIIDA_CONFIG_FOLDER / DEFAULT_DAEMON_DIR_NAME
DAEMON_LOG_DIR: pathlib.Path = DAEMON_DIR / DEFAULT_DAEMON_LOG_DIR_NAME
ACCESS_CONTROL_DIR: pathlib.Path = AIIDA_CONFIG_FOLDER / DEFAULT_ACCESS_CONTROL_DIR_NAME
glb_aiida_config_folder: pathlib.Path = pathlib.Path(DEFAULT_AIIDA_PATH).expanduser() / DEFAULT_CONFIG_DIR_NAME
unkcpz marked this conversation as resolved.
Show resolved Hide resolved


def create_instance_directories() -> None:
@final
unkcpz marked this conversation as resolved.
Show resolved Hide resolved
class AiiDAConfigPathResolver:
"""Path resolver for getting daemon dir, daemon log dir ad access control dir location.
unkcpz marked this conversation as resolved.
Show resolved Hide resolved

If `config_folder` is `None`, `~/.aiida` will be the default root config folder.
unkcpz marked this conversation as resolved.
Show resolved Hide resolved
"""

def __init__(self, config_folder: pathlib.Path | None = None) -> None:
if config_folder is None:
self._aiida_path = glb_aiida_config_folder
else:
self._aiida_path = config_folder

@property
def aiida_path(self) -> pathlib.Path:
return self._aiida_path

@property
def daemon_dir(self) -> pathlib.Path:
return self._aiida_path / DEFAULT_DAEMON_DIR_NAME

@property
def daemon_log_dir(self) -> pathlib.Path:
return self._aiida_path / DEFAULT_DAEMON_DIR_NAME / DEFAULT_DAEMON_LOG_DIR_NAME

@property
def access_control_dir(self) -> pathlib.Path:
return self._aiida_path / DEFAULT_ACCESS_CONTROL_DIR_NAME


def create_instance_directories(aiida_config_folder: pathlib.Path | None) -> None:
"""Create the base directories required for a new AiiDA instance.

This will create the base AiiDA directory defined by the AIIDA_CONFIG_FOLDER variable, unless it already exists.
This will create the base AiiDA directory defined by the glb_aiida_config_folder variable, unless it already exists.
Subsequently, it will create the daemon directory within it and the daemon log directory.
"""
from aiida.common import ConfigurationError

directory_base = AIIDA_CONFIG_FOLDER.expanduser()
directory_daemon = directory_base / DAEMON_DIR
directory_daemon_log = directory_base / DAEMON_LOG_DIR
directory_access = directory_base / ACCESS_CONTROL_DIR
path_resolver = AiiDAConfigPathResolver(aiida_config_folder)

list_of_paths = [
directory_base,
directory_daemon,
directory_daemon_log,
directory_access,
path_resolver.aiida_path,
path_resolver.daemon_dir,
path_resolver.daemon_log_dir,
path_resolver.access_control_dir,
]

umask = os.umask(DEFAULT_UMASK)

try:
for path in list_of_paths:
if path is directory_base and not path.exists():
if path is path_resolver.aiida_path and not path.exists():
warnings.warn(f'Creating AiiDA configuration folder `{path}`.')

try:
path.mkdir(parents=True, exist_ok=True)
except OSError as exc:
raise ConfigurationError(f'could not create the `{path}` configuration directory: {exc}') from exc
finally:
os.umask(umask)
_ = os.umask(umask)


def get_configuration_directory():
Expand All @@ -73,8 +98,8 @@ def get_configuration_directory():
The location of the configuration directory is defined following these heuristics in order:

* If the ``AIIDA_PATH`` variable is set, all the paths will be checked to see if they contain a
configuration folder. The first one to be encountered will be set as ``AIIDA_CONFIG_FOLDER``. If none of them
contain one, the last path defined in the environment variable considered is used.
configuration folder. The first one to be encountered will be set as ``glb_aiida_config_folder``.
If none of them contain one, the last path defined in the environment variable considered is used.
* If an existing directory is still not found, the ``DEFAULT_AIIDA_PATH`` is used.

:returns: The path of the configuration directory.
Expand Down Expand Up @@ -102,6 +127,8 @@ def get_configuration_directory_from_envvar() -> pathlib.Path | None:
if environment_variable is None:
return None

dirpath_config = None

# Loop over all the paths in the ``AIIDA_PATH`` variable to see if any of them contain a configuration folder
for base_dir_path in [path for path in environment_variable.split(':') if path]:
dirpath_config = pathlib.Path(base_dir_path).expanduser()
Expand All @@ -125,17 +152,10 @@ def set_configuration_directory(aiida_config_folder: pathlib.Path | None = None)
is returned by ``get_configuration_directory``. If the directory does not exist yet, it is created, together with
all its subdirectories.
"""
global AIIDA_CONFIG_FOLDER # noqa: PLW0603
global DAEMON_DIR # noqa: PLW0603
global DAEMON_LOG_DIR # noqa: PLW0603
global ACCESS_CONTROL_DIR # noqa: PLW0603

AIIDA_CONFIG_FOLDER = aiida_config_folder or get_configuration_directory()
DAEMON_DIR = AIIDA_CONFIG_FOLDER / DEFAULT_DAEMON_DIR_NAME
DAEMON_LOG_DIR = DAEMON_DIR / DEFAULT_DAEMON_LOG_DIR_NAME
ACCESS_CONTROL_DIR = AIIDA_CONFIG_FOLDER / DEFAULT_ACCESS_CONTROL_DIR_NAME
global glb_aiida_config_folder # noqa: PLW0603
glb_aiida_config_folder = aiida_config_folder or get_configuration_directory()

create_instance_directories()
create_instance_directories(glb_aiida_config_folder)


# Initialize the configuration directory settings
Expand Down
Loading
Loading