Skip to content

Commit

Permalink
Switch to ruamel to keep comments in yaml files
Browse files Browse the repository at this point in the history
PyYaml is a good library, but had the unfortunate side-effect of
stripping comments from yaml files on read/write. Switching to ruamel
now let's us retain the comments, and arguably improves usage in some
ways.
There may be some issues with awxkit objects due to the way the
representers are added, but I can't say for certain that is the case
yet.
  • Loading branch information
JacobCallahan committed Sep 27, 2024
1 parent 0f937ae commit 7bdbd02
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 129 deletions.
33 changes: 12 additions & 21 deletions broker/config_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,14 @@
import click
from logzero import logger
from packaging.version import Version
import yaml
from ruamel.yaml import YAML, YAMLError

from broker import exceptions

yaml = YAML()
yaml.default_flow_style = False
yaml.sort_keys = False

C_SEP = "." # chunk separator
GH_CFG = "https://raw.githubusercontent.com/SatelliteQE/broker/master/broker_settings.yaml.example"

Expand All @@ -25,14 +29,6 @@ def file_name_to_ver(file_name):
return Version(file_name[1:].replace("_", "."))


def yaml_format(data):
"""Format the data as yaml.
Duplicating here to avoid circular imports.
"""
return yaml.dump(data, default_flow_style=False, sort_keys=False)


class ConfigManager:
"""Class to interact with Broker's configuration file.
Expand All @@ -52,7 +48,7 @@ def __init__(self, settings_path=None):
self._settings_path = settings_path
if settings_path:
if settings_path.exists():
self._cfg = yaml.safe_load(self._settings_path.read_text())
self._cfg = yaml.load(self._settings_path)
else:
click.secho(
f"Broker settings file not found at {settings_path.absolute()}.", fg="red"
Expand All @@ -74,15 +70,14 @@ def interactive_mode(self):
def _interactive_edit(self, chunk):
"""Write the chunk data to a temporary file and open it in an editor."""
with NamedTemporaryFile(mode="w+", suffix=".yaml") as tmp:
tmp.write(yaml_format(chunk))
tmp.seek(0)
yaml.dump(chunk, tmp)
click.edit(filename=tmp.name)
tmp.seek(0)
new_data = tmp.read()
# first try to load it as yaml
try:
return yaml.safe_load(new_data)
except yaml.YAMLError: # then try json
return yaml.load(new_data)
except YAMLError: # then try json
try:
return json.loads(new_data)
except json.JSONDecodeError: # finally, just return the raw data
Expand Down Expand Up @@ -187,9 +182,7 @@ def update(self, chunk, new_val, curr_chunk=None):
# update the config file if it exists
if self._settings_path.exists():
self.backup()
self._settings_path.write_text(
yaml.dump(self._cfg, default_flow_style=False, sort_keys=False)
)
yaml.dump(self._cfg, self._settings_path)
else: # we're not at the top level, so keep going down
if C_SEP in chunk:
curr, chunk = chunk.split(C_SEP, 1)
Expand Down Expand Up @@ -237,7 +230,7 @@ def init_config_file(self, chunk=None, _from=None):
raise exceptions.ConfigurationError(
f"Broker settings file not found at {self._settings_path.absolute()}."
)
chunk_data = self.get(chunk, yaml.safe_load(raw_data))
chunk_data = self.get(chunk, yaml.load(raw_data))
if self.interactive_mode:
chunk_data = self._interactive_edit(chunk_data)
self.update(chunk, chunk_data)
Expand All @@ -253,9 +246,7 @@ def migrate(self, force_version=None):
for migration in sorted(migrations, key=lambda m: m.TO_VERSION):
working_config = migration.run_migrations(working_config)
self.backup()
self._settings_path.write_text(
yaml.dump(working_config, default_flow_style=False, sort_keys=False)
)
self._settings_path.write_text(yaml.dump(working_config))
logger.info("Config migration complete.")

def validate(self, chunk, providers=None):
Expand Down
25 changes: 13 additions & 12 deletions broker/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from copy import deepcopy
import getpass
import inspect
from io import BytesIO
import json
import os
from pathlib import Path
Expand All @@ -18,13 +19,17 @@

import click
from logzero import logger
import yaml
from ruamel.yaml import YAML

from broker import exceptions, logger as b_log, settings

FilterTest = namedtuple("FilterTest", "haystack needle test")
INVENTORY_LOCK = threading.Lock()

yaml = YAML()
yaml.default_flow_style = False
yaml.sort_keys = False


def clean_dict(in_dict):
"""Remove entries from a dict where value is None."""
Expand Down Expand Up @@ -167,15 +172,10 @@ def load_file(file, warn=True):
if warn:
logger.warning(f"File {file.absolute()} is invalid or does not exist.")
return []
loader_args = {}
if file.suffix == ".json":
loader = json
return json.loads(file.read_text())
elif file.suffix in (".yaml", ".yml"):
loader = yaml
loader_args = {"Loader": yaml.FullLoader}
with file.open() as f:
data = loader.load(f, **loader_args) or []
return data
return yaml.load(file)


def resolve_file_args(broker_args):
Expand Down Expand Up @@ -251,8 +251,7 @@ def update_inventory(add=None, remove=None):
inv_data.extend(add)

settings.inventory_path.touch()
with settings.inventory_path.open("w") as inv_file:
yaml.dump(inv_data, inv_file)
yaml.dump(inv_data, settings.inventory_path)


def yaml_format(in_struct):
Expand All @@ -263,8 +262,10 @@ def yaml_format(in_struct):
:return: yaml-formatted string
"""
if isinstance(in_struct, str):
in_struct = yaml.load(in_struct, Loader=yaml.FullLoader)
return yaml.dump(in_struct, default_flow_style=False, sort_keys=False)
in_struct = yaml.load(in_struct)
output = BytesIO() # ruamel doesn't natively allow for string output
yaml.dump(in_struct, output)
return output.getvalue().decode("utf-8")


def flip_provider_actions(provider_actions):
Expand Down
13 changes: 8 additions & 5 deletions broker/providers/ansible_tower.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
"""Ansible Tower provider implementation."""

from functools import cache, cached_property
import inspect
import json
Expand All @@ -7,7 +8,7 @@
import click
from dynaconf import Validator
from logzero import logger
import yaml
from ruamel.yaml import YAML

from broker import exceptions
from broker.helpers import eval_filter, find_origin
Expand All @@ -21,6 +22,8 @@
from broker import helpers
from broker.providers import Provider

YAML().register_class(awxkit.utils.PseudoNamespace)


class JobExecutionError(exceptions.ProviderError):
"""Raised when a job execution fails."""
Expand Down Expand Up @@ -778,9 +781,9 @@ def release(self, name, broker_args=None):
)


def awxkit_representer(dumper, data):
"""In order to resolve awxkit objects, a custom representer is needed."""
return dumper.represent_dict(dict(data))
# def awxkit_representer(dumper, data):
# """In order to resolve awxkit objects, a custom representer is needed."""
# return dumper.represent_dict(dict(data))


yaml.add_representer(awxkit.utils.PseudoNamespace, awxkit_representer)
# yaml.add_representer(awxkit.utils.PseudoNamespace, awxkit_representer)
Loading

0 comments on commit 7bdbd02

Please sign in to comment.