From 9a04ad06a0ef38bf1e5ed9964e4f94ac1c4c9def Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 15 Mar 2017 19:32:50 +0100 Subject: [PATCH] Move rate limits out of Config The (legacy) Config object rate limit-related modifications are reverted. For the legacy case, configuration is handled in a specific RateLimits builder class. --- .../cartodb_services/metrics/config.py | 38 ++-------------- .../refactor/config/rate_limits.py | 45 +++++++++++++++++++ .../cartodb_services/tools/service_manager.py | 40 ++++++++--------- 3 files changed, 69 insertions(+), 54 deletions(-) diff --git a/server/lib/python/cartodb_services/cartodb_services/metrics/config.py b/server/lib/python/cartodb_services/cartodb_services/metrics/config.py index 7b44d97..f3e93d0 100644 --- a/server/lib/python/cartodb_services/cartodb_services/metrics/config.py +++ b/server/lib/python/cartodb_services/cartodb_services/metrics/config.py @@ -56,13 +56,6 @@ class ServiceConfig(object): else: return None - def _get_rate_limit(self, service): - rate_limit_key = "{0}_rate_limit".format(service) - rate_limit_json = self._redis_config.get(rate_limit_key, None) - if (rate_limit_json): - return rate_limit_json and json.loads(rate_limit_json) - else: - return self._db_config.rate_limits.get(service, {}) class DataObservatoryConfig(ServiceConfig): @@ -405,8 +398,6 @@ class GeocoderConfig(ServiceConfig): self._cost_per_hit = 0 self._mapzen_service_params = db_config.mapzen_geocoder_service_params - self._rate_limit = self._get_rate_limit('geocoder') - @property def service_type(self): if self._geocoder_provider == self.GOOGLE_GEOCODER: @@ -483,10 +474,6 @@ class GeocoderConfig(ServiceConfig): def provider(self): return self._geocoder_provider - @property - def rate_limit(self): - return self._rate_limit - @property def service(self): return self._service @@ -504,7 +491,6 @@ class ServicesDBConfig: self._get_here_config() self._get_mapzen_config() self._get_data_observatory_config() - self._get_rate_limits_config() def _get_server_config(self): server_config_json = self._get_conf('server_conf') @@ -562,22 +548,14 @@ class ServicesDBConfig: else: self._data_observatory_connection_str = do_conf['connection']['production'] - def _get_rate_limits_config(self): - # self._rate_limits = json.loads(self._get_conf('rate_limits', default='{}')) - rl = self._get_conf('rate_limits', default='{}') - self._rate_limits = json.loads(rl) - def _get_conf(self, key, default=KeyError): + def _get_conf(self, key): try: sql = "SELECT cartodb.CDB_Conf_GetConf('{0}') as conf".format(key) - conf = self._db_conn.execute(sql, 1)[0]['conf'] - if default != KeyError: - conf = conf or default - return conf + conf = self._db_conn.execute(sql, 1) + return conf[0]['conf'] except Exception as e: - if (default == KeyError): - raise ConfigException("Error trying to get config for {0}: {1}".format(key, e)) - return default + raise ConfigException("Error trying to get config for {0}: {1}".format(key, e)) @property def server_environment(self): @@ -655,10 +633,6 @@ class ServicesDBConfig: def data_observatory_connection_str(self): return self._data_observatory_connection_str - @property - def rate_limits(self): - return self._rate_limits - @property def logger_config(self): logger_conf_json = self._get_conf('logger_conf') @@ -681,7 +655,6 @@ class ServicesRedisConfig: GEOCODER_PROVIDER_KEY = 'geocoder_provider' ISOLINES_PROVIDER_KEY = 'isolines_provider' ROUTING_PROVIDER_KEY = 'routing_provider' - GEOCODING_RATE_LIMIT_KEY = 'geocoder_rate_limit' def __init__(self, redis_conn): self._redis_connection = redis_conn @@ -736,6 +709,3 @@ class ServicesRedisConfig: user_config[self.ISOLINES_PROVIDER_KEY] = org_config[self.ISOLINES_PROVIDER_KEY] if self.ROUTING_PROVIDER_KEY in org_config: user_config[self.ROUTING_PROVIDER_KEY] = org_config[self.ROUTING_PROVIDER_KEY] - # for rate limit parameters, user config has precedence over organization - if self.GEOCODING_RATE_LIMIT_KEY in org_config and not self.GEOCODING_RATE_LIMIT_KEY in user_config: - user_config[self.GEOCODING_RATE_LIMIT_KEY] = org_config[self.GEOCODING_RATE_LIMIT_KEY] diff --git a/server/lib/python/cartodb_services/cartodb_services/refactor/config/rate_limits.py b/server/lib/python/cartodb_services/cartodb_services/refactor/config/rate_limits.py index 70bde8b..6a465f5 100644 --- a/server/lib/python/cartodb_services/cartodb_services/refactor/config/rate_limits.py +++ b/server/lib/python/cartodb_services/cartodb_services/refactor/config/rate_limits.py @@ -63,3 +63,48 @@ class RateLimitsConfigBuilder(object): rate_limit.get('limit', None), rate_limit.get('period', None)) +class RateLimitsConfigLegacyBuilder(object): + """ + Build a RateLimitsConfig object using the legacy configuration + classes ... + instead of the refactored ... + """ + + def __init__(self, redis_connection, db_conn, service, user, org): + self._service = service + self._username = user + self._orgname = org + self._redis_connection = redis_connection + self._db_conn = db_conn + + def get(self): + rate_limit = self.__get_rate_limit() + return RateLimitsConfig(self._service, + self._username, + rate_limit.get('limit', None), + rate_limit.get('period', None)) + + def __get_rate_limit(self): + rate_limit = {} + rate_limit_key = "{0}_rate_limit".format(self._service) + user_key = "rails:users:{0}".format(self._username) + rate_limit_json = self.__get_redis_config(user_key, rate_limit_key) + if not rate_limit_json and self._orgname: + org_key = "rails:orgs:{0}".format(self._orgname) + rate_limit_json = self.__get_redis_config(org_key, rate_limit_key) + if rate_limit_json: + rate_limit = json.loads(rate_limit_json) + else: + conf_key = 'rate_limits' + sql = "SELECT cartodb.CDB_Conf_GetConf('{0}') as conf".format(conf_key) + try: + conf = self._db_conn.execute(sql, 1)[0]['conf'] + except Exception: + conf = None + if conf: + rate_limit = json.loads(conf).get(self._service) + return rate_limit or {} + + def __get_redis_config(self, basekey, param): + config = self._redis_connection.hgetall(basekey) + return config and config.get(param) diff --git a/server/lib/python/cartodb_services/cartodb_services/tools/service_manager.py b/server/lib/python/cartodb_services/cartodb_services/tools/service_manager.py index 4ac76cb..5f140a5 100644 --- a/server/lib/python/cartodb_services/cartodb_services/tools/service_manager.py +++ b/server/lib/python/cartodb_services/cartodb_services/tools/service_manager.py @@ -19,26 +19,6 @@ class ServiceManagerBase: def logger(self): return self.logger -from cartodb_services.metrics import QuotaService -from cartodb_services.tools import Logger,LoggerConfig -from cartodb_services.tools import RateLimiter -from cartodb_services.refactor.config.rate_limits import RateLimitsConfig - -class LegacyServiceManager(ServiceManagerBase): - - def __init__(self, service, username, orgname, gd): - redis_conn = gd["redis_connection_{0}".format(username)]['redis_metrics_connection'] - self.config = gd["user_{0}_config_{1}".format(service, username)] - logger_config = gd["logger_config"] - self.logger = Logger(logger_config) - - rate_limits_config = RateLimitsConfig(service, - username, - self.config.rate_limit.get('limit'), - self.config.rate_limit.get('period')) - self.rate_limiter = RateLimiter(rate_limits_config, redis_conn) - self.quota_service = QuotaService(self.config, redis_conn) - from cartodb_services.metrics import QuotaService from cartodb_services.tools import Logger from cartodb_services.tools import RateLimiter @@ -68,3 +48,23 @@ class ServiceManager(ServiceManagerBase): self.rate_limiter = RateLimiter(rate_limit_config, redis_metrics_connection) self.quota_service = QuotaService(self.config, redis_metrics_connection) + +from cartodb_services.metrics import QuotaService +from cartodb_services.tools import Logger,LoggerConfig +from cartodb_services.tools import RateLimiter +from cartodb_services.refactor.config.rate_limits import RateLimitsConfigLegacyBuilder +import plpy + +class LegacyServiceManager(ServiceManagerBase): + + def __init__(self, service, username, orgname, gd): + redis_conn = gd["redis_connection_{0}".format(username)]['redis_metrics_connection'] + self.config = gd["user_{0}_config_{1}".format(service, username)] + logger_config = gd["logger_config"] + self.logger = Logger(logger_config) + + self.quota_service = QuotaService(self.config, redis_conn) + + rate_limit_config = RateLimitsConfigLegacyBuilder(redis_conn, plpy, service=service, user=username, org=orgname).get() + self.rate_limiter = RateLimiter(rate_limit_config, redis_conn) +