Added CR suggestions

This commit is contained in:
Antonio 2018-01-04 12:30:58 +01:00
parent cc34a8b19b
commit 51d97228dc
18 changed files with 103 additions and 14732 deletions

File diff suppressed because it is too large Load Diff

File diff suppressed because it is too large Load Diff

View File

@ -450,7 +450,7 @@ $$ LANGUAGE 'plpgsql' SECURITY DEFINER STABLE PARALLEL UNSAFE;
-- These are the only ones with permissions to publicuser role
-- and should also be the only ones with SECURITY DEFINER
CREATE OR REPLACE FUNCTION cdb_dataservices_client.cdb_mapbox_geocode_street_point (searchtext text ,city text DEFAULT NULL ,state_province text DEFAULT NULL ,country text DEFAULT NULL)
CREATE OR REPLACE FUNCTION cdb_dataservices_client.cdb_mapbox_geocode_street_point (searchtext text, city text DEFAULT NULL, state_province text DEFAULT NULL, country text DEFAULT NULL)
RETURNS Geometry AS $$
DECLARE
ret Geometry;
@ -550,7 +550,7 @@ $$ LANGUAGE 'plpgsql' SECURITY DEFINER STABLE PARALLEL UNSAFE;
-- These are the only ones with permissions to publicuser role
-- and should also be the only ones with SECURITY DEFINER
CREATE OR REPLACE FUNCTION cdb_dataservices_client.cdb_mapbox_isochrone (source geometry(Geometry, 4326) ,mode text ,range integer[] ,options text[] DEFAULT ARRAY[]::text[])
CREATE OR REPLACE FUNCTION cdb_dataservices_client.cdb_mapbox_isochrone (source geometry(Geometry, 4326), mode text, range integer[], options text[] DEFAULT ARRAY[]::text[])
RETURNS SETOF cdb_dataservices_client.isoline AS $$
DECLARE
@ -600,7 +600,7 @@ $$ LANGUAGE 'plpgsql' SECURITY DEFINER STABLE PARALLEL UNSAFE;
-- These are the only ones with permissions to publicuser role
-- and should also be the only ones with SECURITY DEFINER
CREATE OR REPLACE FUNCTION cdb_dataservices_client.cdb_mapbox_isodistance (source geometry(Geometry, 4326) ,mode text ,range integer[] ,options text[] DEFAULT ARRAY[]::text[])
CREATE OR REPLACE FUNCTION cdb_dataservices_client.cdb_mapbox_isodistance (source geometry(Geometry, 4326), mode text, range integer[], options text[] DEFAULT ARRAY[]::text[])
RETURNS SETOF cdb_dataservices_client.isoline AS $$
DECLARE
@ -2374,7 +2374,7 @@ $$ LANGUAGE 'plpgsql' SECURITY DEFINER STABLE PARALLEL UNSAFE;
-- Exception-safe private DataServices API function
--
CREATE OR REPLACE FUNCTION cdb_dataservices_client._cdb_mapbox_geocode_street_point_exception_safe (searchtext text ,city text DEFAULT NULL ,state_province text DEFAULT NULL ,country text DEFAULT NULL)
CREATE OR REPLACE FUNCTION cdb_dataservices_client._cdb_mapbox_geocode_street_point_exception_safe (searchtext text, city text DEFAULT NULL, state_province text DEFAULT NULL, country text DEFAULT NULL)
RETURNS Geometry AS $$
DECLARE
ret Geometry;
@ -2518,7 +2518,7 @@ $$ LANGUAGE 'plpgsql' SECURITY DEFINER STABLE PARALLEL UNSAFE;
-- Exception-safe private DataServices API function
--
CREATE OR REPLACE FUNCTION cdb_dataservices_client._cdb_mapbox_isochrone_exception_safe (source geometry(Geometry, 4326) ,mode text ,range integer[] ,options text[] DEFAULT ARRAY[]::text[])
CREATE OR REPLACE FUNCTION cdb_dataservices_client._cdb_mapbox_isochrone_exception_safe(source geometry(Geometry, 4326), mode text, range integer[], options text[] DEFAULT ARRAY[]::text[])
RETURNS SETOF cdb_dataservices_client.isoline AS $$
DECLARE
@ -2590,7 +2590,7 @@ $$ LANGUAGE 'plpgsql' SECURITY DEFINER STABLE PARALLEL UNSAFE;
-- Exception-safe private DataServices API function
--
CREATE OR REPLACE FUNCTION cdb_dataservices_client._cdb_mapbox_isodistance_exception_safe (source geometry(Geometry, 4326) ,mode text ,range integer[] ,options text[] DEFAULT ARRAY[]::text[])
CREATE OR REPLACE FUNCTION cdb_dataservices_client._cdb_mapbox_isodistance_exception_safe(source geometry(Geometry, 4326), mode text, range integer[], options text[] DEFAULT ARRAY[]::text[])
RETURNS SETOF cdb_dataservices_client.isoline AS $$
DECLARE

File diff suppressed because it is too large Load Diff

File diff suppressed because it is too large Load Diff

View File

@ -2013,7 +2013,7 @@ CREATE OR REPLACE FUNCTION cdb_dataservices_server._cdb_mapzen_geocode_street_po
RETURNS Geometry AS $$
from cartodb_services.tools import ServiceManager
from cartodb_services.mapzen import MapzenGeocoder
from cartodb_services.mapzen.types import country_to_iso3
from cartodb_services.tools.country import country_to_iso3
from cartodb_services.refactor.service.mapzen_geocoder_config import MapzenGeocoderConfigBuilder
import cartodb_services
@ -2409,7 +2409,7 @@ RETURNS Geometry AS $$
from cartodb_services.mapbox import MapboxGeocoder
from cartodb_services.metrics import QuotaService, metrics
from cartodb_services.tools import Logger,LoggerConfig
from cartodb_services.mapzen.types import country_to_iso3
from cartodb_services.tools.country import country_to_iso3
plpy.execute("SELECT cdb_dataservices_server._connect_to_redis('{0}')".format(username))
redis_conn = GD["redis_connection_{0}".format(username)]['redis_metrics_connection']
@ -2454,7 +2454,7 @@ $$ LANGUAGE plpythonu STABLE PARALLEL RESTRICTED;
CREATE OR REPLACE FUNCTION cdb_dataservices_server._cdb_mapzen_geocode_namedplace(username text, orgname text, city_name text, admin1_name text DEFAULT NULL, country_name text DEFAULT NULL)
RETURNS Geometry AS $$
from cartodb_services.mapzen import MapzenGeocoder
from cartodb_services.mapzen.types import country_to_iso3
from cartodb_services.tools.country import country_to_iso3
from cartodb_services.metrics import QuotaService, metrics
from cartodb_services.tools import Logger,LoggerConfig

View File

@ -4,7 +4,7 @@ import re
from requests.adapters import HTTPAdapter
from exceptions import WrongParams, MalformedResult, ServiceException
from qps import qps_retry
from cartodb_services.tools.qps import qps_retry
from cartodb_services.tools import Coordinate, PolyLine
from cartodb_services.metrics import Traceable

View File

@ -4,7 +4,7 @@ import re
from requests.adapters import HTTPAdapter
from exceptions import WrongParams, MalformedResult, ServiceException
from qps import qps_retry
from cartodb_services.tools.qps import qps_retry
class MapzenIsochrones:

View File

@ -1,6 +1,6 @@
import requests
import json
from qps import qps_retry
from cartodb_services.tools.qps import qps_retry
from exceptions import ServiceException
from cartodb_services.metrics import Traceable

View File

@ -1,69 +0,0 @@
import time
import random
from datetime import datetime
from exceptions import TimeoutException
DEFAULT_RETRY_TIMEOUT = 60
DEFAULT_QUERIES_PER_SECOND = 10
def qps_retry(original_function=None,**options):
""" Query Per Second retry decorator
The intention of this decorator is to retry requests against third
party services that has QPS restriction.
Parameters:
- timeout: Maximum number of seconds to retry
- qps: Allowed queries per second. This parameter is used to
calculate the next time to retry the request
"""
if original_function is not None:
def wrapped_function(*args, **kwargs):
if 'timeout' in options:
timeout = options['timeout']
else:
timeout = DEFAULT_RETRY_TIMEOUT
if 'qps' in options:
qps = options['qps']
else:
qps = DEFAULT_QUERIES_PER_SECOND
return QPSService(retry_timeout=timeout, queries_per_second=qps).call(original_function, *args, **kwargs)
return wrapped_function
else:
def partial_wrapper(func):
return qps_retry(func, **options)
return partial_wrapper
class QPSService:
def __init__(self, queries_per_second, retry_timeout):
self._queries_per_second = queries_per_second
self._retry_timeout = retry_timeout
def call(self, fn, *args, **kwargs):
start_time = datetime.now()
attempt_number = 1
while True:
try:
return fn(*args, **kwargs)
except Exception as e:
response = getattr(e, 'response', None)
if response is not None and (response.status_code == 429):
self.retry(start_time, attempt_number)
else:
raise e
attempt_number += 1
def retry(self, first_request_time, retry_count):
elapsed = datetime.now() - first_request_time
if elapsed.total_seconds() > self._retry_timeout:
raise TimeoutException()
# inverse qps * (1.5 ^ i) is an increased sleep time of 1.5x per
# iteration.
delay = (1.0/self._queries_per_second) * 1.5 ** retry_count
# https://www.awsarchitectureblog.com/2015/03/backoff.html
# https://github.com/googlemaps/google-maps-services-python/blob/master/googlemaps/client.py#L193
sleep_time = delay * (random.random() + 0.5)
time.sleep(sleep_time)

View File

@ -4,7 +4,7 @@ import re
from requests.adapters import HTTPAdapter
from exceptions import WrongParams, MalformedResult, ServiceException
from qps import qps_retry
from cartodb_services.tools.qps import qps_retry
from cartodb_services.tools import Coordinate, PolyLine
from cartodb_services.metrics import MetricsDataGatherer, Traceable

View File

@ -35,18 +35,3 @@ def coordinates_to_polygon(coordinates):
geometry = None
return geometry
def country_to_iso3(country):
""" Convert country to its iso3 code """
try:
country_plan = plpy.prepare("SELECT adm0_a3 as iso3 FROM admin0_synonyms WHERE lower(regexp_replace($1, " \
"'[^a-zA-Z\u00C0-\u00ff]+', '', 'g'))::text = name_; ", ['text'])
country_result = plpy.execute(country_plan, [country], 1)
if country_result:
return country_result[0]['iso3']
else:
return None
except BaseException as e:
plpy.warning("Can't get the iso3 code from {0}: {1}".format(country, e))
return None

View File

@ -599,19 +599,19 @@ class ServicesDBConfig:
def _get_mapbox_config(self):
mapbox_conf_json = self._get_conf('mapbox_conf')
if not mapbox_conf_json:
raise ConfigException('Mapzen configuration missing')
raise ConfigException('Mapbox configuration missing')
else:
mapzen_conf = json.loads(mapbox_conf_json)
self._mapbox_matrix_api_key = mapzen_conf['matrix']['api_key']
self._mapbox_matrix_quota = mapzen_conf['matrix']['monthly_quota']
self._mapbox_matrix_service_params = mapzen_conf['matrix'].get('service', {})
self._mapbox_isochrones_service_params = mapzen_conf.get('isochrones', {}).get('service', {})
self._mapbox_routing_api_key = mapzen_conf['routing']['api_key']
self._mapbox_routing_quota = mapzen_conf['routing']['monthly_quota']
self._mapbox_routing_service_params = mapzen_conf['routing'].get('service', {})
self._mapbox_geocoder_api_key = mapzen_conf['geocoder']['api_key']
self._mapbox_geocoder_quota = mapzen_conf['geocoder']['monthly_quota']
self._mapbox_geocoder_service_params = mapzen_conf['geocoder'].get('service', {})
mapbox_conf = json.loads(mapbox_conf_json)
self._mapbox_matrix_api_key = mapbox_conf['matrix']['api_key']
self._mapbox_matrix_quota = mapbox_conf['matrix']['monthly_quota']
self._mapbox_matrix_service_params = mapbox_conf['matrix'].get('service', {})
self._mapbox_isochrones_service_params = mapbox_conf.get('isochrones', {}).get('service', {})
self._mapbox_routing_api_key = mapbox_conf['routing']['api_key']
self._mapbox_routing_quota = mapbox_conf['routing']['monthly_quota']
self._mapbox_routing_service_params = mapbox_conf['routing'].get('service', {})
self._mapbox_geocoder_api_key = mapbox_conf['geocoder']['api_key']
self._mapbox_geocoder_quota = mapbox_conf['geocoder']['monthly_quota']
self._mapbox_geocoder_service_params = mapbox_conf['geocoder'].get('service', {})
def _get_data_observatory_config(self):
do_conf_json = self._get_conf('data_observatory_conf')

View File

@ -72,6 +72,9 @@ class QuotaChecker:
if re.match('geocoder_*',
self._user_service_config.service_type) is not None:
return self.__check_geocoder_quota()
elif re.match('here_isolines',
self._user_service_config.service_type) is not None:
return self.__check_isolines_quota()
elif re.match('mapzen_isolines',
self._user_service_config.service_type) is not None:
return self.__check_isolines_quota()

View File

@ -1,3 +1,3 @@
class TimeoutException(Exception):
def __str__(self):
return repr('Timeout requesting to mapzen server')
return repr('Timeout requesting to server')

View File

@ -6,7 +6,7 @@ rollbar==0.13.2
# Dependency for googlemaps package
requests==2.9.1
rratelimit==0.0.4
mapbox
mapbox==0.14.0
# Test
mock==1.3.0

View File

@ -10,7 +10,7 @@ from setuptools import setup, find_packages
setup(
name='cartodb_services',
version='0.15.7',
version='0.16.0',
description='CartoDB Services API Python Library',

View File

@ -114,29 +114,64 @@ class TestQuotaService(TestCase):
qs.increment_success_service_use(amount=1500000)
assert qs.check_user_quota() is False
def test_should_check_user_routing_quota_correctly(self):
qs = self.__build_routing_quota_service('test_user')
def test_should_check_user_mapzen_routing_quota_correctly(self):
qs = self.__build_routing_quota_service('test_user', provider='mapzen')
qs.increment_success_service_use()
assert qs.check_user_quota() is True
qs.increment_success_service_use(amount=1500000)
assert qs.check_user_quota() is False
def test_should_check_org_routing_quota_correctly(self):
qs = self.__build_routing_quota_service('test_user', orgname='testorg')
def test_should_check_org_mapzen_routing_quota_correctly(self):
qs = self.__build_routing_quota_service('test_user', provider='mapzen',
orgname='testorg')
qs.increment_success_service_use()
assert qs.check_user_quota() is True
qs.increment_success_service_use(amount=1500000)
assert qs.check_user_quota() is False
def test_should_check_user_isolines_quota_correctly(self):
qs = self.__build_isolines_quota_service('test_user')
def test_should_check_user_mapbox_routing_quota_correctly(self):
qs = self.__build_routing_quota_service('test_user', provider='mapbox')
qs.increment_success_service_use()
assert qs.check_user_quota() is True
qs.increment_success_service_use(amount=1500000)
assert qs.check_user_quota() is False
def test_should_check_org_mapbox_routing_quota_correctly(self):
qs = self.__build_routing_quota_service('test_user', provider='mapbox',
orgname='testorg')
qs.increment_success_service_use()
assert qs.check_user_quota() is True
qs.increment_success_service_use(amount=1500000)
assert qs.check_user_quota() is False
def test_should_check_user_mapzen_isolines_quota_correctly(self):
qs = self.__build_isolines_quota_service('test_user',
provider='mapzen')
qs.increment_isolines_service_use()
assert qs.check_user_quota() is True
qs.increment_isolines_service_use(amount=1500000)
assert qs.check_user_quota() is False
def test_should_check_org_isolines_quota_correctly(self):
def test_should_check_org_mapzen_isolines_quota_correctly(self):
qs = self.__build_isolines_quota_service('test_user',
provider='mapzen',
orgname='testorg')
qs.increment_isolines_service_use()
assert qs.check_user_quota() is True
qs.increment_isolines_service_use(amount=1500000)
assert qs.check_user_quota() is False
def test_should_check_user_mapbox_isolines_quota_correctly(self):
qs = self.__build_isolines_quota_service('test_user',
provider='mapbox')
qs.increment_isolines_service_use()
assert qs.check_user_quota() is True
qs.increment_isolines_service_use(amount=1500000)
assert qs.check_user_quota() is False
def test_should_check_org_mapbox_isolines_quota_correctly(self):
qs = self.__build_isolines_quota_service('test_user',
provider='mapbox',
orgname='testorg')
qs.increment_isolines_service_use()
assert qs.check_user_quota() is True
@ -196,7 +231,7 @@ class TestQuotaService(TestCase):
username, orgname)
return QuotaService(geocoder_config, redis_connection=self.redis_conn)
def __build_routing_quota_service(self, username, provider='mapbox',
def __build_routing_quota_service(self, username, provider,
orgname=None, soft_limit=False,
quota=100, end_date=datetime.today()):
self.__prepare_quota_service(username, 'routing', quota, provider,
@ -205,7 +240,7 @@ class TestQuotaService(TestCase):
username, orgname)
return QuotaService(routing_config, redis_connection=self.redis_conn)
def __build_isolines_quota_service(self, username, provider='mapbox',
def __build_isolines_quota_service(self, username, provider,
orgname=None, soft_limit=False,
quota=100, end_date=datetime.today()):
self.__prepare_quota_service(username, 'isolines', quota, provider,