Skip to content

Commit

Permalink
Some bugfixes and cleanup for actor config code.
Browse files Browse the repository at this point in the history
* Make normalize_schemas() public.  The caller (normally in the workflow) will read the schemas and
  then pass them to load() so it also needs to call normalize_schema() in between those two steps.
* Emit separate warnings for unknown section names and unknown field names
* Remove usage of added_field variable which was replaced in a refactor with using the dictionary of
  schemas (which have the same information as keys of nested dictionaries).
* Remove some functions that have been rewritten after confirming that all of their functionality
  has been ported.
* Add a comment specifically for what is happening with format_config() (Not needed for MVP).
* Remove config_schemas from the RepositoryManager class.  Instead, we will have any caller extract
  that information from RepositoryManager.actors.
* Don't warn for every field in a section that isn't recognized.
* Add both section and field to warning messages about unknown fields.
* Make use of field instead of getting it from the schema dict again.
* Fix name of the validate_field_type function.
  • Loading branch information
abadger committed Sep 12, 2024
1 parent 476d5bd commit 613b399
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 57 deletions.
84 changes: 33 additions & 51 deletions leapp/actors/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ def _get_config(config_dir='/etc/leapp/actor_conf.d'):
return configuration


def _normalize_schemas(schemas):
def normalize_schemas(schemas):
"""
Merge all schemas into a single dictionary and validate them for errors we can detect.
"""
Expand All @@ -152,8 +152,13 @@ def _normalize_schemas(schemas):

# Error if the field has been added by another schema
if unique_name in added_fields and added_fields[unique_name] != field:
# TODO: Also have information on what Actor contains the conflicting fields
message = "Two actors added incompatible values for {name}".format(name=unique_name))
# TODO: Also include information on what Actor contains the
# conflicting fields but that information isn't passed into
# this function right now.
message = ('Two actors added incompatible configuration items'
' with the same name for Section: {section},'
' Field: {field}'.format(section=field.section,
name=field.name))
log.error(message)
raise SchemaError(message)

Expand Down Expand Up @@ -181,15 +186,28 @@ def _validate_field_type(field_type, field_value):


def _normalize_config(actor_config, schema):
# Validate that the config values read from the config files obey the known
# structure.
for section_name, section in actor_config.items():
if section_name not in schema:
# TODO: Also have information about which config file contains the unknown field.
message = "A config file contained an unknown section: {section}".format(section=section_name)
log.warning(message)
continue

for field_name in actor_config:
if (section_name, field_name) not in added_fields:
if field_name not in schema[section_name]:
# TODO: Also have information about which config file contains the unknown field.
message = "A config file contained an unknown field: {name}".format(name=(section_name, field_name))
message = ("A config file contained an unknown field: (Section:"
" {section}, Field: {field})".format(
section=section_name, field=field_name)
)
log.warning(message)

# Do several things:
# * Validate that the config values are of the proper types.
# * Add default values where no config value was provided.
normalized_actor_config = {}

for section_name, section in schema.items():
for field_name, field in section.items():
# TODO: We might be able to do this using the default piece of
Expand All @@ -209,10 +227,14 @@ def _normalize_config(actor_config, schema):
# May need to deepcopy default if these values are modified.
# However, it's probably an error if they are modified and we
# should possibly look into disallowing that.
value = section[field_name] = schema[section_name][field_name].default
value = section[field_name] = field.default

if not _validate_field(schema[section_name][field_name].type_, value):
raise ValidationError("Config value for {name} is not of the correct type".format(name=(section_name, field_name)))
if not _validate_field_type(field.type_, value):
raise ValidationError("Config value for (Section: {section},"
" Field: {field}) is not of the correct"
" type".format(section=section_name,
field=field_name)
)

normalized_section = normalized_actor_config.get(section_name, {})
normalized_section[field_name] = value
Expand All @@ -236,9 +258,6 @@ def load(config_dir, schemas):
if _ACTOR_CONFIG:
return _ACTOR_CONFIG

# TODO: Move this to the caller
schema = _normalize_schemas(schemas)
# End TODO
config = _get_config(config_dir)
config = _normalize_config(config, schema)

Expand All @@ -264,47 +283,10 @@ def retrieve_config(schema):
return configuration

#
# From this point down hasn't been re-evaluated to see if it's needed or how it
# fits into the bigger picture. Some of it definitely has been implemented in
# a different way above but not all of it.
# The function after this needs some work to be ready. It isn't part of the
# upgrade or preupgrade workflows so we don't have to get it finished yet.
#

def parse_repo_config_files():
repo_config = {}
for config in all_repository_config_schemas():
section_name = config.section

if section_name not in repo_config:
repo_config.update(config.to_dict())
else:
if '{0}_description__'.format(config_item.name) in repo_config[config.section]:
raise Exception("Error: Two configuration items are declared with the same name Section: {0}, Key: {1}".format(config.section, config.name))

repo_config[config.section].update(config.to_dict()[config.section])

return repo_config


def parse_config_files(config_dir):
"""
Parse all configuration and return a dict with those values.
"""
config = parse_repo_config_files()
system_config = parse_system_config_files(config_dir)

for section, config_items in system_config.items():
if section not in config:
print('WARNING: config file contains an unused section: Section: {0}'.format(section))
config.update[section] = config_items
else:
for key, value in config_items:
if '{0}_description__'.format(key) not in config[section]:
print('WARNING: config file contains an unused config entry: Section: {0}, Key{1}'.format(section, key))

config[section][key] = value

return config


def format_config():
"""
Expand Down
6 changes: 0 additions & 6 deletions leapp/repository/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,12 +139,6 @@ def actors(self):
"""
return tuple(itertools.chain(*[repo.actors for repo in self._repos.values()]))

@property
def config_schemas(self):
"""
"""
return tuple(itertools.chain(*[repo.config_schemas for repo in self._repos.values()]))

@property
def topics(self):
"""
Expand Down

0 comments on commit 613b399

Please sign in to comment.