apply review suggestions

leapp/actors/__init__.py

@@ -381,10 +381,8 @@ def report_error(self, message, severity=ErrorSeverity.ERROR, details=None):
 
     def retrieve_config(self):
         """
-        Retrieve the configuration specific to the specified schema.
+        Retrieve the configuration described by self.config_schema.
 
-        :param schema: Configuration schemas
-        :type schema: :py:class:`leapp.models.Model`
         :return: Dictionary containing requested configuration.
         :rtype: dict
         """
@@ -426,7 +424,7 @@ def _is_foo_sequence(cls, cls_name, actor, name, value):
         _lint_warn(actor, name, cls_name)
         value = (value,)
     _is_type(Sequence)(actor, name, value)
-    if not all([True] + [isinstance(item, type) and issubclass(item, cls) for item in value]):
+    if not all(isinstance(item, type) and issubclass(item, cls) for item in value):
         raise WrongAttributeTypeError(
             'Actor {} attribute {} should contain only {}'.format(actor, name, cls_name))
     return value

leapp/actors/config.py

@@ -38,7 +38,6 @@
 
 
 _ACTOR_CONFIG = None
-_ACTOR_CONFIG_VALIDATED = False
 
 log = logging.getLogger('leapp.actors.config')
 
@@ -143,7 +142,9 @@ def _get_config(config_dir='/etc/leapp/actor_conf.d'):
     """
     Read all configuration files from the config_dir and return a dict with their values.
     """
-    config_files = glob.glob(os.path.join(config_dir, '*'), recursive=True)
+    # We do not do a recursive walk to maintain Python2 compatibility, but we do
+    # not expect rich config hierarchies, so no harm done.
+    config_files = glob.glob(os.path.join(config_dir, '*'))
     config_files = [f for f in config_files if f.endswith('.yml') or f.endswith('.yaml')]
     config_files.sort()
 
@@ -203,21 +204,18 @@ def _validate_field_type(field_type, field_value, field_path):
     :param str field_path: Path in the config where the field is placed.
                            Example: A field 'target_clients' in a section 'rhui' would have a path 'rhui.target_clients'
     """
-    # TODO: I took a quick look at the Model code and this is what I came up
-    # with.  This might not work right or there might be a much better way.
     try:
         # the name= parameter is displayed in error messages to let the user know what precisely is wrong
         field_type._validate_model_value(field_value, name=field_path)
-    except ModelViolationError as e:  # pylint: disable=broad-exception-caught,broad-except
+    except ModelViolationError as e:
         # Any problems mean that the field did not validate.
         log.info("Configuration value failed to validate with: %s", e)
         return False
     return True
 
 
 def _normalize_config(actor_config, schema):
-    # Validate that the config values read from the config files obey the known
-    # structure.
+    # Go through the config and log warnings about unexpected sections/fields.
     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.
@@ -317,37 +315,3 @@ def retrieve_config(schema):
         configuration[field.section][field.name] = _ACTOR_CONFIG[field.section][field.name]
 
     return dict(configuration)
-
-#
-# 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 format_config():
-    """
-    Read the configuration definitions from all of the known repositories and return a string that
-    can be used as an example config file.
-
-    Example config file:
-        transaction:
-            to_install_description__: |
-                List of packages to be added to the upgrade transaction.
-                Signed packages which are already installed will be skipped.
-            to_remove_description__: |
-                List of packages to be removed from the upgrade transaction
-                initial-setup should be removed to avoid it asking for EULA acceptance during upgrade
-            to_remove:
-                - initial-setup
-            to_keep_description__: |
-                List of packages to be kept in the upgrade transaction
-            to_keep:
-                - leapp
-                - python2-leapp
-                - python3-leapp
-                - leapp-repository
-                - snactor
-    """
-    # TODO: This is just a placeholder.  We need to do some additional
-    # formatting that includes the documentation, not just return it as is.
-    return yaml.dump(_ACTOR_CONFIG, SafeDumper)

leapp/configs/common/__init__.py

@@ -2,6 +2,3 @@
 :py:mod:`leapp.configs.common` represents an import location for shared libraries that
 are placed in the repository's configs folder.
 """
-
-LEAPP_BUILTIN_COMMON_CONFIGS_INITIALIZED = False
-""" Framework internal - were common configs initialized? """

leapp/models/fields/__init__.py

@@ -476,7 +476,7 @@ def __init__(self, value_type, **kwargs):
             self._default = copy.copy(self._default)
 
         if not isinstance(value_type, Field):
-            raise ModelMisuseError("elem_field must be an instance of a type derived from Field")
+            raise ModelMisuseError("value_type must be an instance of a type derived from Field")
 
         self._value_type = value_type