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

config: allow reading teuthology config from env var location #1998

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
30 changes: 23 additions & 7 deletions teuthology/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@
from collections import MutableMapping


# Configuration constants
_SYSTEM_CONFIG_PATH = '/etc/teuthology.yaml'
_USER_CONFIG_PATH = '~/.teuthology.yaml'
_ALT_PATH_ENV_VAR = 'TEUTHOLOGY_CONFIG'

Comment on lines +11 to +14
Copy link
Contributor

Choose a reason for hiding this comment

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

According to PEP8 there is no need to add _ (underscore) in the beginning of the constant name, may we have just SYSTEM_CONFIG_PATH and USER_CONFIG_PATH?
Also, I think to keep similar naming to env, like CONFIG_PATH_ENV_NAME.


def init_logging():
log = logging.getLogger(__name__)
return log
Expand Down Expand Up @@ -135,9 +141,9 @@ class TeuthologyConfig(YamlConfig):
"""
This class is intended to unify teuthology's many configuration files and
objects. Currently it serves as a convenient interface to
~/.teuthology.yaml and nothing else.
~/.teuthology.yaml or equivalent.
"""
yaml_path = os.path.join(os.path.expanduser('~/.teuthology.yaml'))
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess if drop this line you have to update the comment above about "interface to ~/.teuthology.yaml and nothing else".

yaml_path = _USER_CONFIG_PATH # yaml_path is updated in _get_config_path
_defaults = {
'archive_base': '/home/teuthworker/archive',
'archive_upload': None,
Expand Down Expand Up @@ -285,10 +291,20 @@ def set_config_attr(obj):


def _get_config_path():
system_config_path = '/etc/teuthology.yaml'
if not os.path.exists(TeuthologyConfig.yaml_path) and \
os.path.exists(system_config_path):
return system_config_path
return TeuthologyConfig.yaml_path
"""Look for a teuthology config yaml and return it's path.
Raises ValueError if no config yaml can be found.
"""
paths = [
os.path.join(os.path.expanduser(_USER_CONFIG_PATH)),
_SYSTEM_CONFIG_PATH,
]
if _ALT_PATH_ENV_VAR in os.environ:
paths.insert(0, os.path.expanduser(os.environ[_ALT_PATH_ENV_VAR]))
for path in paths:
if os.path.exists(path):
return path
log.warning(f"no teuthology config found, looked for: {paths}")
return None
Copy link
Contributor

@kshtsk kshtsk Aug 26, 2024

Choose a reason for hiding this comment

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

The idea was to always return a meaningful value, see usage of config constructor below.



config = TeuthologyConfig(yaml_path=_get_config_path())