From 87bb168772eca2203a2a5ba426e808dd173c4939 Mon Sep 17 00:00:00 2001 From: Adrian Gaudebert Date: Thu, 24 Nov 2011 02:38:54 +0100 Subject: [PATCH] Added one config key per service. Removed hard-coded implementation order. Changed the get_module algorithm accordingly. --- scripts/config/webapiconfig.py.dist | 43 ++++++++++--- socorro/middleware/search_service.py | 4 -- socorro/middleware/service.py | 70 ++++++++------------- socorro/unittest/middleware/test_service.py | 20 +++--- 4 files changed, 70 insertions(+), 67 deletions(-) diff --git a/scripts/config/webapiconfig.py.dist b/scripts/config/webapiconfig.py.dist index 84fbb93f84..93f4f18bd5 100644 --- a/scripts/config/webapiconfig.py.dist +++ b/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' diff --git a/socorro/middleware/search_service.py b/socorro/middleware/search_service.py index eb2b642c1d..81eaabd18b 100644 --- a/socorro/middleware/search_service.py +++ b/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/([^/.]*)/(.*)" diff --git a/socorro/middleware/service.py b/socorro/middleware/service.py index db6e4c5ee6..6c7bc11575 100644 --- a/socorro/middleware/service.py +++ b/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): """ diff --git a/socorro/unittest/middleware/test_service.py b/socorro/unittest/middleware/test_service.py index 6f2baddf27..2997e35cdf 100644 --- a/socorro/unittest/middleware/test_service.py +++ b/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