Add exception for non-supported key in systems configuration (#236)

* Add exception for non-supported key in system config

Before this change, if the user didn't use a valid key
in one of the systems' configuration sections, the exception
thrown was generic and didn't tell the user much on what
exactly the issue is.

With this change, the user will know which key is not supported
and for what type of system.

* Modify jenkins and elasticsearch source for jobs_scope

After jobs_scope was moved out of the JobsSystem API, the sources needed
to be updated to use jobs_scope as a string and not an Argument object.

Co-authored-by: jgilaber <[email protected]>

cibyl/exceptions/config.py

@@ -73,3 +73,13 @@ def __init__(self, source_type, key):
 is not supported: {key}\n\n{CHECK_DOCS_MSG}"""
 
         super().__init__(self.message)
+
+
+class NonSupportedSystemKey(CibylException):
+    """Configuration section key is not supported."""
+
+    def __init__(self, source_type, key):
+        self.message = f"""The following key in "{source_type}" system type \
+is not supported: {key}\n\n{CHECK_DOCS_MSG}"""
+
+        super().__init__(self.message)

cibyl/exceptions/model.py

@@ -14,6 +14,7 @@
 #    under the License.
 """
 from cibyl.exceptions import CibylException
+from cibyl.exceptions.config import CHECK_DOCS_MSG
 from cibyl.utils.colors import Colors
 
 
@@ -31,15 +32,14 @@ class NoValidSystem(CibylException):
     """Exception for a case when no valid system is found."""
 
     def __init__(self, valid_systems=None):
-        self.message = """No valid system defined."""
+        self.message = """No valid system(s) defined."""
         if valid_systems:
             self.message += "\nPlease use one of the following available"
             self.message += " systems:\n"
             for system_name in valid_systems:
                 self.message += f"  {Colors.blue(system_name)}\n"
         else:
-            self.message += "\nPlease ensure the specified systems are present"
-            self.message += " in the configuration."
+            self.message += f"\n{CHECK_DOCS_MSG}"
         super().__init__(self.message)
 
 

cibyl/models/ci/base/system.py

@@ -71,8 +71,7 @@ def __init__(self, name: str,
                  system_type: str,
                  top_level_model: Type[Model],
                  sources: List = None,
-                 enabled: bool = True,
-                 **kwargs):
+                 enabled: bool = True):
         # Let IDEs know this class's attributes
         self.name = None
         self.system_type = None
@@ -87,8 +86,7 @@ def __init__(self, name: str,
                 'system_type': system_type,
                 'sources': sources,
                 'enabled': enabled,
-                'queried': False,
-                **kwargs
+                'queried': False
             }
         )
 
@@ -167,17 +165,14 @@ class JobsSystem(System):
     """Model a system with :class:`Job` as its top-level model.
     """
     API = deepcopy(System.API)
-    API.update({'jobs_scope': {'attr_type': str,
-                               'arguments': []},
-                'jobs': {
-                    'attr_type': Job,
-                    'attribute_value_class': AttributeDictValue,
-                    'arguments': [
-                        Argument(name='--jobs', arg_type=str,
-                                 nargs='*',
-                                 description="System jobs",
-                                 func='get_jobs')]
-    }})
+    API.update({
+        'jobs': {
+            'attr_type': Job,
+            'attribute_value_class': AttributeDictValue,
+            'arguments': [Argument(name='--jobs', arg_type=str,
+                                   nargs='*',
+                                   description="System jobs",
+                                   func='get_jobs')]}})
 
     def __init__(self,
                  name: str,
@@ -186,9 +181,8 @@ def __init__(self,
                  enabled: bool = True,
                  jobs: Dict[str, Job] = None,
                  jobs_scope: str = None):
-        # Let IDEs know this class's attributes
-        self.jobs = None
-        self.jobs_scope = None
+        self.jobs = jobs
+        self.jobs_scope = jobs_scope
 
         # Set up model
         super().__init__(
@@ -197,8 +191,6 @@ def __init__(self,
             top_level_model=Job,
             sources=sources,
             enabled=enabled,
-            jobs_scope=jobs_scope,
-            jobs=jobs
         )
 
     def export_attributes_to_source(self):

cibyl/models/ci/system_factory.py

@@ -18,7 +18,6 @@
 from cibyl.exceptions import CibylNotImplementedException
 from cibyl.models.ci.base.system import JobsSystem
 from cibyl.models.ci.zuul.system import ZuulSystem
-from cibyl.utils.dicts import subset
 
 
 class SystemType(str, Enum):
@@ -46,17 +45,11 @@ def create_system(system_type, name, **kwargs):
         """
         system_type = system_type.lower()
 
-        # Extract common arguments of interest
-        args = subset(kwargs, ['sources', 'enabled'])
-
         if system_type == SystemType.JENKINS:
-            # Add arguments specific for this type
-            args.update(subset(kwargs, ['jobs_scope']))
-
-            return JobsSystem(name=name, system_type=system_type, **args)
+            return JobsSystem(name=name, system_type=system_type, **kwargs)
 
         if system_type == SystemType.ZUUL:
-            return ZuulSystem(name=name, system_type=system_type, **args)
+            return ZuulSystem(name=name, system_type=system_type, **kwargs)
 
         msg = f"Unknown system type '{system_type}'"
         raise CibylNotImplementedException(msg)

cibyl/models/ci/zuul/system.py

@@ -46,8 +46,7 @@ def __init__(self,
                  sources=None,
                  enabled=True,
                  tenants=None):
-        # Let IDEs know this class's attributes
-        self.tenants = None
+        self.tenants = tenants
 
         # Set up model
         super().__init__(
@@ -56,7 +55,6 @@ def __init__(self,
             top_level_model=Tenant,
             sources=sources,
             enabled=enabled,
-            tenants=tenants
         )
 
     def add_toplevel_model(self, model):

cibyl/orchestrator.py

@@ -15,6 +15,7 @@
 """
 import logging
 import operator
+import re
 import time
 from collections import defaultdict
 from copy import deepcopy
@@ -23,6 +24,7 @@
 from cibyl.cli.parser import Parser
 from cibyl.cli.validator import Validator
 from cibyl.config import Config, ConfigFactory
+from cibyl.exceptions.config import NonSupportedSystemKey
 from cibyl.exceptions.source import (NoSupportedSourcesFound, NoValidSources,
                                      SourceException)
 from cibyl.models.ci.base.environment import Environment
@@ -91,6 +93,20 @@ def get_source(self, source_name, source_data):
             raise conf_exc.InvalidSourceConfiguration(
                 source_name, source_data) from exception
 
+    def add_system_to_environment(self, environment,
+                                  system_name, sources, single_system):
+        try:
+            environment.add_system(
+                name=system_name,
+                sources=sources,
+                **single_system
+            )
+        except TypeError as ex:
+            re_result = re.search(r'unexpected keyword argument (.*)',
+                                  ex.args[0])
+            if re_result:
+                raise NonSupportedSystemKey(system_name, re_result.group(1))
+
     def create_ci_environments(self) -> None:
         """Creates CI environment entities based on loaded configuration."""
         try:
@@ -110,11 +126,8 @@ def create_ci_environments(self) -> None:
                         sources.append(
                             self.get_source(source_name, source_data))
 
-                    environment.add_system(
-                        name=system_name,
-                        sources=sources,
-                        **single_system
-                    )
+                    self.add_system_to_environment(environment, system_name,
+                                                   sources, single_system)
 
                 self.environments.append(environment)
             self.set_system_api()

cibyl/sources/elasticsearch/api.py

@@ -80,8 +80,8 @@ def get_jobs(self: object, **kwargs: Argument) -> list:
             jobs_to_search = kwargs.get('job_url').value
             key_filter = 'job_url'
         jobs_scope_arg = kwargs.get('jobs_scope')
-        if jobs_scope_arg and jobs_scope_arg.value:
-            jobs_scope_pattern = re.compile(jobs_scope_arg.value)
+        if jobs_scope_arg:
+            jobs_scope_pattern = re.compile(jobs_scope_arg)
 
         query_body = QueryTemplate(key_filter, jobs_to_search).get
 

cibyl/sources/jenkins.py

@@ -158,8 +158,8 @@ def filter_jobs(jobs_found: List[Dict], **kwargs):
                                        field_to_check="name"))
 
     jobs_scope_arg = kwargs.get('jobs_scope')
-    if jobs_scope_arg and jobs_scope_arg.value:
-        pattern = re.compile(jobs_scope_arg.value)
+    if jobs_scope_arg:
+        pattern = re.compile(jobs_scope_arg)
         checks_to_apply.append(partial(satisfy_regex_match, pattern=pattern,
                                        field_to_check="name"))
 

tests/unit/models/ci/base/test_system.py

@@ -98,4 +98,4 @@ def test_export_attributes_to_source(self):
         """Test system export_attributes_to_source method."""
         output = self.system.export_attributes_to_source()
         self.assertEqual(1, len(output))
-        self.assertEqual(output['jobs_scope'].value, 'phase1')
+        self.assertEqual(output['jobs_scope'], 'phase1')

tests/unit/sources/elasticsearch/test_api.py

@@ -150,9 +150,7 @@ def test_get_jobs_jobs_scope(self: object, mock_query_hits: object):
         """
         mock_query_hits.return_value = self.job_hits
 
-        jobs_argument = Mock()
-        jobs_argument.value = 'test4'
-        jobs = self.es_api.get_jobs(jobs_scope=jobs_argument)
+        jobs = self.es_api.get_jobs(jobs_scope='test4')
 
         self.assertEqual(len(jobs), 1)
         self.assertTrue('test4' in jobs)

tests/unit/sources/test_jenkins.py

@@ -2107,9 +2107,7 @@ def test_filter_jobs_scope(self):
                     {'_class': 'org..job.WorkflowRun',
                      'name': "ans2", 'url': 'url3',
                      'lastBuild': {'number': 0, 'result': "FAILURE"}}]
-        args = Mock()
-        args.value = "ans"
-        jobs_filtered = filter_jobs(response, jobs_scope=args)
+        jobs_filtered = filter_jobs(response, jobs_scope="ans")
         expected = [{'_class': 'org..job.WorkflowRun',
                      'name': "ansible", 'url': 'url1',
                      'lastBuild': {'number': 1, 'result': "SUCCESS"}},