Added one config key per service. Removed hard-coded implementation o…

…rder. Changed the get_module algorithm accordingly.

scripts/config/webapiconfig.py.dist

@@ -30,12 +30,6 @@ wsgiInstallation = cm.Option()
 wsgiInstallation.doc = 'True or False, this app is installed under WSGI'
 wsgiInstallation.default = True
 
-# Implementation class for the Middleware services implementations
-# Can be socorro.external.elasticsearch or socorro.external.postgresql
-serviceImplementationModule = cm.Option()
-serviceImplementationModule.doc = 'String, name of the module that implements the services.'
-serviceImplementationModule.default = 'socorro.external.postgresql'
-
 # Elastic Search configuration
 elasticSearchHostname = cm.Option()
 elasticSearchHostname.doc = 'String containing the URI of the Elastic Search instance.'
@@ -45,6 +39,23 @@ elasticSearchPort = cm.Option()
 elasticSearchPort.doc = 'String containing the port on which calling the Elastic Search instance.'
 elasticSearchPort.default = '9200'
 
+#---------------------------------------------------------------------------
+# Configuration for middleware services
+
+# Default implementation class for the Middleware services implementations
+# If a module doesn't define it's own value, use that one.
+# Can be socorro.external.elasticsearch or socorro.external.postgresql
+serviceImplementationModule = cm.Option()
+serviceImplementationModule.doc = ("String, name of the default module that "
+                                   "services use.")
+serviceImplementationModule.default = "socorro.external.postgresql"
+
+# Search service config
+searchImplementationModule = cm.Option()
+searchImplementationModule.doc = ("String, name of the module the search "
+                                  "service uses.")
+searchImplementationModule.default = 'socorro.external.postgresql'
+
 searchMaxNumberOfDistinctSignatures = cm.Option()
 searchMaxNumberOfDistinctSignatures.doc = (
         "Integer containing the maximum allowed number of distinct signatures "
@@ -72,7 +83,25 @@ import socorro.middleware.search_service as search
 
 servicesList = cm.Option()
 servicesList.doc = 'a python list of classes to offer as services'
-servicesList.default = [search.Search, tcbst.TopCrashBySignatureTrends, sighist.SignatureHistory, adubd.AduByDay, adudetails.AduByDayDetails, crash.GetCrash, emailcampaign.EmailCampaign, emailcreate.EmailCampaignCreate, emaillist.EmailCampaigns, emailvolume.EmailCampaignVolume, emailsub.EmailSubscription, emailsend.EmailSender, schedule.SchedulePriorityJob, bugzilla.Bugzilla, cv.CurrentVersions, vi.VersionsInfo, hr.HangReport]
+servicesList.default = [
+    tcbst.TopCrashBySignatureTrends,
+    sighist.SignatureHistory,
+    adubd.AduByDay,
+    adudetails.AduByDayDetails,
+    crash.GetCrash,
+    emailcampaign.EmailCampaign,
+    emailcreate.EmailCampaignCreate,
+    emaillist.EmailCampaigns,
+    emailvolume.EmailCampaignVolume,
+    emailsub.EmailSubscription,
+    emailsend.EmailSender,
+    schedule.SchedulePriorityJob,
+    bugzilla.Bugzilla,
+    cv.CurrentVersions,
+    vi.VersionsInfo,
+    hr.HangReport,
+    search.Search,
+]
 
 crashBaseUrl = cm.Option()
 crashBaseUrl.doc = 'The base url for linking to crash-stats. This will be used in email templates'

socorro/middleware/search_service.py

@@ -15,10 +15,6 @@ class Search(DataAPIService):
 
     """
 
-    default_service_order = [
-        "socorro.external.postgresql",
-        "socorro.external.elasticsearch"
-    ]
     service_name = "search"
     uri = "/search/([^/.]*)/(.*)"
 

socorro/middleware/service.py

@@ -1,5 +1,6 @@
 import logging
 import sys
+import web
 
 from socorro.webapi.webapiService import JsonWebServiceBase
 
@@ -15,7 +16,6 @@ class DataAPIService(JsonWebServiceBase):
 
     """
 
-    default_service_order = []
     service_name = ""
 
     def __init__(self, config):
@@ -34,63 +34,45 @@ def get_module(self, params):
 
         Find the external module that will be called by the service to execute
         the required action. If one exists and is valid, use user input first,
-        then configuration, then default_service_order of the service.
+        then this service's configuration, then default services'
+        configuration.
 
-        Raise a NotImplementedError if no implementation was found.
+        Raise a 400 Bad Request HTTP error if user forced the implementation
+        to a value that does not exist.
 
-        Return the imported module.
+        Raise an internal error if configuration is improper.
 
-        """
-        impl = None
+        Return the imported module otherwise.
 
-        # First use user value if it exists
+        """
         if "force_api_impl" in params:
             module_name = ".".join(("socorro.external",
                                     params["force_api_impl"],
                                     self.service_name))
-            impl = self._import(module_name)
-
-        if impl:
-            logger.debug("Service %s uses forced implementation module: %s" %
-                         (self.service_name, module_name))
-            return impl
-
-        # Second use config value
-        module_name = "%s.%s" % (self.context.serviceImplementationModule,
-                                 self.service_name)
-        impl = self._import(module_name)
-
-        if impl:
-            logger.debug("Service %s uses config module: %s" %
-                         (self.service_name, module_name))
-            return impl
-
-        # Third use module values in order of preference
-        for m in self.default_service_order:
-            module_name = "%s.%s" % (m, self.service_name)
-            impl = self._import(module_name)
-            if impl:
-                logger.debug("Service %s uses default module: %s" %
-                             (self.service_name, module_name))
-                return impl
-
-        # No implementation was found, raise an error
-        raise NotImplementedError
-
-    def _import(self, module_name):
-        """
-        Import a module, check it exists and return it.
-
-        Return the module if it exists, False otherwise.
+            try:
+                __import__(module_name)
+                return sys.modules[module_name]
+            except ImportError:
+                logger.debug("Could not import %s" % module_name)
+                raise web.webapi.BadRequest()
+
+        service_config_key = "%sImplementationModule" % self.service_name
+        if service_config_key in self.context:
+            # Use that specific service's config value
+            module_name = "%s.%s" % (self.context[service_config_key],
+                                     self.service_name)
+        else:
+            # Use the generic services' config value
+            module_name = "%s.%s" % (self.context.serviceImplementationModule,
+                                     self.service_name)
 
-        """
-        logger.debug("Try to import %s" % module_name)
         try:
             __import__(module_name)
             return sys.modules[module_name]
         except ImportError:
             logger.debug("Could not import %s" % module_name)
-            return False
+            raise web.webapi.InternalError(message=("Improper configuration, could not find "
+                                     "module %s" % module_name))
 
     def parse_query_string(self, query_string):
         """

socorro/unittest/middleware/test_service.py

@@ -20,6 +20,7 @@ def get_dummy_context():
     context.databaseUserName = 'ricky'
     context.databasePassword = 'lucy'
     context.databasePort = 127
+    context.searchImplementationModule = "socorro.external.postgresql"
     context.serviceImplementationModule = "socorro.external.elasticsearch"
     context.elasticSearchHostname = "localhost"
     context.elasticSearchPort = "9200"
@@ -34,7 +35,7 @@ def test_get_module():
     service.service_name = "search"
     params = {}
 
-    # Test config module
+    # Test service config module
     import_failed = False
     try:
         mod = service.get_module(params)
@@ -50,12 +51,12 @@ def test_get_module():
     except AttributeError:
         assert False, "The imported module does not contain the needed class"
 
-    assert isinstance(search, es.ElasticSearchBase), (
+    assert isinstance(search, pg.PostgreSQLBase), (
                 "Imported module is not the right one")
 
     # Test forced module
     import_failed = False
-    params["force_api_impl"] = "postgresql"
+    params["force_api_impl"] = "elasticsearch"
     try:
         mod = service.get_module(params)
     except NotImplementedError:
@@ -70,17 +71,13 @@ def test_get_module():
     except AttributeError:
         assert False, "The imported module does not contain the needed class"
 
-    assert isinstance(search, pg.PostgreSQLBase), (
+    assert isinstance(search, es.ElasticSearchBase), (
                 "Imported module is not the right one")
 
-    # Test default module
+    # Test default config module
     import_failed = False
     params = {}
-    service.default_service_order = [
-        "socorro.external.elasticsearch",
-        "socorro.external.postgresql",
-    ]
-    config.serviceImplementationModule = "unknownmodule"
+    del config.searchImplementationModule
     try:
         mod = service.get_module(params)
     except NotImplementedError:
@@ -101,11 +98,10 @@ def test_get_module():
     # Test no valid module to import
     import_failed = False
     params = {}
-    service.default_service_order = []
     config.serviceImplementationModule = "unknownmodule"
     try:
         mod = service.get_module(params)
-    except NotImplementedError:
+    except AttributeError: # catching the exception raised by web.InternalError
         import_failed = True
 
     assert import_failed, "Impossible import succeeded: %s" % mod