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
12 changes: 7 additions & 5 deletions teuthology/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ class TeuthologyConfig(YamlConfig):
objects. Currently it serves as a convenient interface to
~/.teuthology.yaml and nothing else.
"""
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".

_defaults = {
'archive_base': '/home/teuthworker/archive',
'archive_upload': None,
Expand Down Expand Up @@ -285,10 +284,13 @@ def set_config_attr(obj):


def _get_config_path():
config_path = os.environ.get('TEUTHOLOGY_CONFIG', '~/.teuthology.yaml')
config_path = os.path.join(os.path.expanduser(config_path))
Copy link
Contributor

Choose a reason for hiding this comment

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

Bad style, overriding variable.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also if we declare the env for the config, we should stop immediately if such file does not exists.

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
for path in (config_path, system_config_path):
if os.path.exists(path):
return path
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())
Loading