Added CR suggestions

server/extension/cdb_dataservices_server--0.29.0--0.30.0.sql

@@ -11,7 +11,7 @@ CREATE OR REPLACE FUNCTION cdb_dataservices_server._cdb_mapbox_route_with_waypoi
 RETURNS cdb_dataservices_server.simple_route AS $$
   import json
   from cartodb_services.mapbox import MapboxRouting, MapboxRoutingResponse
-  from cartodb_services.mapbox.types import MODE_TO_MAPBOX_PROFILE
+  from cartodb_services.mapbox.types import TRANSPORT_MODE_TO_MAPBOX
   from cartodb_services.metrics import QuotaService
   from cartodb_services.tools import Coordinate
   from cartodb_services.tools import Logger,LoggerConfig
@@ -47,7 +47,7 @@ RETURNS cdb_dataservices_server.simple_route AS $$
       lon = plpy.execute("SELECT ST_X('%s') AS lon" % waypoint)[0]['lon']
       waypoint_coords.append(Coordinate(lon,lat))
 
-    profile = MODE_TO_MAPBOX_PROFILE.get(mode)
+    profile = TRANSPORT_MODE_TO_MAPBOX.get(mode)
 
     resp = client.directions(waypoint_coords, profile)
     if resp and resp.shape:
@@ -92,9 +92,17 @@ RETURNS cdb_dataservices_server.simple_route AS $$
 
   with metrics('cdb_route_with_point', user_routing_config, logger):
     waypoints = [origin, destination]
+
+    if user_routing_config.mapzen_provider:
+      mapzen_plan = plpy.prepare("SELECT * FROM cdb_dataservices_server._cdb_mapzen_route_with_waypoints($1, $2, $3, $4) as route;", ["text", "text", "geometry(Point, 4326)[]", "text"])
+      result = plpy.execute(mapzen_plan, [username, orgname, waypoints, mode])
+      return [result[0]['shape'],result[0]['length'], result[0]['duration']]
+    elif user_routing_config.mapbox_provider:
       mapbox_plan = plpy.prepare("SELECT * FROM cdb_dataservices_server._cdb_mapbox_route_with_waypoints($1, $2, $3, $4) as route;", ["text", "text", "geometry(Point, 4326)[]", "text"])
       result = plpy.execute(mapbox_plan, [username, orgname, waypoints, mode])
       return [result[0]['shape'],result[0]['length'], result[0]['duration']]
+    else:
+      raise Exception('Requested routing method is not available')
 $$ LANGUAGE plpythonu STABLE PARALLEL RESTRICTED;
 
 CREATE OR REPLACE FUNCTION cdb_dataservices_server.cdb_route_with_waypoints(
@@ -117,9 +125,16 @@ RETURNS cdb_dataservices_server.simple_route AS $$
   logger = Logger(logger_config)
 
   with metrics('cdb_route_with_waypoints', user_routing_config, logger):
+    if user_routing_config.mapzen_provider:
+      mapzen_plan = plpy.prepare("SELECT * FROM cdb_dataservices_server._cdb_mapzen_route_with_waypoints($1, $2, $3, $4) as route;", ["text", "text", "geometry(Point, 4326)[]", "text"])
+      result = plpy.execute(mapzen_plan, [username, orgname, waypoints, mode])
+      return [result[0]['shape'],result[0]['length'], result[0]['duration']]
+    elif user_routing_config.mapbox_provider:
       mapbox_plan = plpy.prepare("SELECT * FROM cdb_dataservices_server._cdb_mapbox_route_with_waypoints($1, $2, $3, $4) as route;", ["text", "text", "geometry(Point, 4326)[]", "text"])
       result = plpy.execute(mapbox_plan, [username, orgname, waypoints, mode])
       return [result[0]['shape'],result[0]['length'], result[0]['duration']]
+    else:
+      raise Exception('Requested routing method is not available')
 $$ LANGUAGE plpythonu STABLE PARALLEL RESTRICTED;
 
 CREATE OR REPLACE FUNCTION cdb_dataservices_server.cdb_geocode_street_point(username TEXT, orgname TEXT, searchtext TEXT, city TEXT DEFAULT NULL, state_province TEXT DEFAULT NULL, country TEXT DEFAULT NULL)
@@ -222,7 +237,7 @@ RETURNS Geometry AS $$
   if not quota_service.check_user_quota():
     raise Exception('You have reached the limit of your quota')
 
-  with metrics('cdb_geocode_namedplace_point', user_geocoder_config, logger):
+  with metrics('cdb_mapbox_geocode_street_point', user_geocoder_config, logger):
     try:
       geocoder = MapboxGeocoder(user_geocoder_config.mapbox_api_key, logger, user_geocoder_config.mapbox_service_params)
 
@@ -388,7 +403,7 @@ CREATE OR REPLACE FUNCTION cdb_dataservices_server._cdb_mapbox_isodistance(
 RETURNS SETOF cdb_dataservices_server.isoline AS $$
   import json
   from cartodb_services.mapbox import MapboxMatrixClient, MapboxIsolines
-  from cartodb_services.mapbox.types import MODE_TO_MAPBOX_PROFILE
+  from cartodb_services.mapbox.types import TRANSPORT_MODE_TO_MAPBOX
   from cartodb_services.tools import Coordinate
   from cartodb_services.metrics import QuotaService
   from cartodb_services.tools import Logger,LoggerConfig
@@ -414,7 +429,7 @@ RETURNS SETOF cdb_dataservices_server.isoline AS $$
     else:
       raise Exception('source is NULL')
 
-    profile = MODE_TO_MAPBOX_PROFILE.get(mode)
+    profile = TRANSPORT_MODE_TO_MAPBOX.get(mode)
 
     # -- TODO Support options properly
     isolines = {}
@@ -458,7 +473,7 @@ CREATE OR REPLACE FUNCTION cdb_dataservices_server._cdb_mapbox_isochrones(
 RETURNS SETOF cdb_dataservices_server.isoline AS $$
   import json
   from cartodb_services.mapbox import MapboxMatrixClient, MapboxIsolines
-  from cartodb_services.mapbox.types import MODE_TO_MAPBOX_PROFILE
+  from cartodb_services.mapbox.types import TRANSPORT_MODE_TO_MAPBOX
   from cartodb_services.tools import Coordinate
   from cartodb_services.tools.coordinates import coordinates_to_polygon
   from cartodb_services.metrics import QuotaService
@@ -486,7 +501,7 @@ RETURNS SETOF cdb_dataservices_server.isoline AS $$
     else:
       raise Exception('source is NULL')
 
-    profile = MODE_TO_MAPBOX_PROFILE.get(mode)
+    profile = TRANSPORT_MODE_TO_MAPBOX.get(mode)
 
     resp = mapbox_isolines.calculate_isochrone(origin, data_range, profile)
 

server/extension/cdb_dataservices_server--0.30.0.sql

@@ -15,7 +15,7 @@ CREATE OR REPLACE FUNCTION cdb_dataservices_server._cdb_mapbox_route_with_waypoi
 RETURNS cdb_dataservices_server.simple_route AS $$
   import json
   from cartodb_services.mapbox import MapboxRouting, MapboxRoutingResponse
-  from cartodb_services.mapbox.types import MODE_TO_MAPBOX_PROFILE
+  from cartodb_services.mapbox.types import TRANSPORT_MODE_TO_MAPBOX
   from cartodb_services.metrics import QuotaService
   from cartodb_services.tools import Coordinate
   from cartodb_services.tools import Logger,LoggerConfig
@@ -51,7 +51,7 @@ RETURNS cdb_dataservices_server.simple_route AS $$
       lon = plpy.execute("SELECT ST_X('%s') AS lon" % waypoint)[0]['lon']
       waypoint_coords.append(Coordinate(lon,lat))
 
-    profile = MODE_TO_MAPBOX_PROFILE.get(mode)
+    profile = TRANSPORT_MODE_TO_MAPBOX.get(mode)
 
     resp = client.directions(waypoint_coords, profile)
     if resp and resp.shape:
@@ -156,9 +156,17 @@ RETURNS cdb_dataservices_server.simple_route AS $$
 
   with metrics('cdb_route_with_point', user_routing_config, logger):
     waypoints = [origin, destination]
+
+    if user_routing_config.mapzen_provider:
+      mapzen_plan = plpy.prepare("SELECT * FROM cdb_dataservices_server._cdb_mapzen_route_with_waypoints($1, $2, $3, $4) as route;", ["text", "text", "geometry(Point, 4326)[]", "text"])
+      result = plpy.execute(mapzen_plan, [username, orgname, waypoints, mode])
+      return [result[0]['shape'],result[0]['length'], result[0]['duration']]
+    elif user_routing_config.mapbox_provider:
       mapbox_plan = plpy.prepare("SELECT * FROM cdb_dataservices_server._cdb_mapbox_route_with_waypoints($1, $2, $3, $4) as route;", ["text", "text", "geometry(Point, 4326)[]", "text"])
       result = plpy.execute(mapbox_plan, [username, orgname, waypoints, mode])
       return [result[0]['shape'],result[0]['length'], result[0]['duration']]
+    else:
+      raise Exception('Requested routing method is not available')
 $$ LANGUAGE plpythonu STABLE PARALLEL RESTRICTED;
 
 
@@ -182,9 +190,16 @@ RETURNS cdb_dataservices_server.simple_route AS $$
   logger = Logger(logger_config)
 
   with metrics('cdb_route_with_waypoints', user_routing_config, logger):
+    if user_routing_config.mapzen_provider:
+      mapzen_plan = plpy.prepare("SELECT * FROM cdb_dataservices_server._cdb_mapzen_route_with_waypoints($1, $2, $3, $4) as route;", ["text", "text", "geometry(Point, 4326)[]", "text"])
+      result = plpy.execute(mapzen_plan, [username, orgname, waypoints, mode])
+      return [result[0]['shape'],result[0]['length'], result[0]['duration']]
+    elif user_routing_config.mapbox_provider:
       mapbox_plan = plpy.prepare("SELECT * FROM cdb_dataservices_server._cdb_mapbox_route_with_waypoints($1, $2, $3, $4) as route;", ["text", "text", "geometry(Point, 4326)[]", "text"])
       result = plpy.execute(mapbox_plan, [username, orgname, waypoints, mode])
       return [result[0]['shape'],result[0]['length'], result[0]['duration']]
+    else:
+      raise Exception('Requested routing method is not available')
 $$ LANGUAGE plpythonu STABLE PARALLEL RESTRICTED;
 -- Get the connection to redis from cache or create a new one
 CREATE OR REPLACE FUNCTION cdb_dataservices_server._connect_to_redis(user_id text)
@@ -3069,7 +3084,7 @@ CREATE OR REPLACE FUNCTION cdb_dataservices_server._cdb_mapbox_isodistance(
 RETURNS SETOF cdb_dataservices_server.isoline AS $$
   import json
   from cartodb_services.mapbox import MapboxMatrixClient, MapboxIsolines
-  from cartodb_services.mapbox.types import MODE_TO_MAPBOX_PROFILE
+  from cartodb_services.mapbox.types import TRANSPORT_MODE_TO_MAPBOX
   from cartodb_services.tools import Coordinate
   from cartodb_services.metrics import QuotaService
   from cartodb_services.tools import Logger,LoggerConfig
@@ -3095,7 +3110,7 @@ RETURNS SETOF cdb_dataservices_server.isoline AS $$
     else:
       raise Exception('source is NULL')
 
-    profile = MODE_TO_MAPBOX_PROFILE.get(mode)
+    profile = TRANSPORT_MODE_TO_MAPBOX.get(mode)
 
     # -- TODO Support options properly
     isolines = {}
@@ -3202,7 +3217,7 @@ CREATE OR REPLACE FUNCTION cdb_dataservices_server._cdb_mapbox_isochrones(
 RETURNS SETOF cdb_dataservices_server.isoline AS $$
   import json
   from cartodb_services.mapbox import MapboxMatrixClient, MapboxIsolines
-  from cartodb_services.mapbox.types import MODE_TO_MAPBOX_PROFILE
+  from cartodb_services.mapbox.types import TRANSPORT_MODE_TO_MAPBOX
   from cartodb_services.tools import Coordinate
   from cartodb_services.tools.coordinates import coordinates_to_polygon
   from cartodb_services.metrics import QuotaService
@@ -3230,7 +3245,7 @@ RETURNS SETOF cdb_dataservices_server.isoline AS $$
     else:
       raise Exception('source is NULL')
 
-    profile = MODE_TO_MAPBOX_PROFILE.get(mode)
+    profile = TRANSPORT_MODE_TO_MAPBOX.get(mode)
 
     resp = mapbox_isolines.calculate_isochrone(origin, data_range, profile)
 

server/extension/sql/100_routing_helper.sql

@@ -12,7 +12,7 @@ CREATE OR REPLACE FUNCTION cdb_dataservices_server._cdb_mapbox_route_with_waypoi
 RETURNS cdb_dataservices_server.simple_route AS $$
   import json
   from cartodb_services.mapbox import MapboxRouting, MapboxRoutingResponse
-  from cartodb_services.mapbox.types import MODE_TO_MAPBOX_PROFILE
+  from cartodb_services.mapbox.types import TRANSPORT_MODE_TO_MAPBOX
   from cartodb_services.metrics import QuotaService
   from cartodb_services.tools import Coordinate
   from cartodb_services.tools import Logger,LoggerConfig
@@ -48,7 +48,7 @@ RETURNS cdb_dataservices_server.simple_route AS $$
       lon = plpy.execute("SELECT ST_X('%s') AS lon" % waypoint)[0]['lon']
       waypoint_coords.append(Coordinate(lon,lat))
 
-    profile = MODE_TO_MAPBOX_PROFILE.get(mode)
+    profile = TRANSPORT_MODE_TO_MAPBOX.get(mode)
 
     resp = client.directions(waypoint_coords, profile)
     if resp and resp.shape:

server/extension/sql/105_route_between_points.sql

@@ -20,9 +20,17 @@ RETURNS cdb_dataservices_server.simple_route AS $$
 
   with metrics('cdb_route_with_point', user_routing_config, logger):
     waypoints = [origin, destination]
+
+    if user_routing_config.mapzen_provider:
+      mapzen_plan = plpy.prepare("SELECT * FROM cdb_dataservices_server._cdb_mapzen_route_with_waypoints($1, $2, $3, $4) as route;", ["text", "text", "geometry(Point, 4326)[]", "text"])
+      result = plpy.execute(mapzen_plan, [username, orgname, waypoints, mode])
+      return [result[0]['shape'],result[0]['length'], result[0]['duration']]
+    elif user_routing_config.mapbox_provider:
       mapbox_plan = plpy.prepare("SELECT * FROM cdb_dataservices_server._cdb_mapbox_route_with_waypoints($1, $2, $3, $4) as route;", ["text", "text", "geometry(Point, 4326)[]", "text"])
       result = plpy.execute(mapbox_plan, [username, orgname, waypoints, mode])
       return [result[0]['shape'],result[0]['length'], result[0]['duration']]
+    else:
+      raise Exception('Requested routing method is not available')
 $$ LANGUAGE plpythonu STABLE PARALLEL RESTRICTED;
 
 
@@ -46,7 +54,14 @@ RETURNS cdb_dataservices_server.simple_route AS $$
   logger = Logger(logger_config)
 
   with metrics('cdb_route_with_waypoints', user_routing_config, logger):
+    if user_routing_config.mapzen_provider:
+      mapzen_plan = plpy.prepare("SELECT * FROM cdb_dataservices_server._cdb_mapzen_route_with_waypoints($1, $2, $3, $4) as route;", ["text", "text", "geometry(Point, 4326)[]", "text"])
+      result = plpy.execute(mapzen_plan, [username, orgname, waypoints, mode])
+      return [result[0]['shape'],result[0]['length'], result[0]['duration']]
+    elif user_routing_config.mapbox_provider:
       mapbox_plan = plpy.prepare("SELECT * FROM cdb_dataservices_server._cdb_mapbox_route_with_waypoints($1, $2, $3, $4) as route;", ["text", "text", "geometry(Point, 4326)[]", "text"])
       result = plpy.execute(mapbox_plan, [username, orgname, waypoints, mode])
       return [result[0]['shape'],result[0]['length'], result[0]['duration']]
+    else:
+      raise Exception('Requested routing method is not available')
 $$ LANGUAGE plpythonu STABLE PARALLEL RESTRICTED;

server/extension/sql/80_isolines_helper.sql

@@ -133,7 +133,7 @@ CREATE OR REPLACE FUNCTION cdb_dataservices_server._cdb_mapbox_isodistance(
 RETURNS SETOF cdb_dataservices_server.isoline AS $$
   import json
   from cartodb_services.mapbox import MapboxMatrixClient, MapboxIsolines
-  from cartodb_services.mapbox.types import MODE_TO_MAPBOX_PROFILE
+  from cartodb_services.mapbox.types import TRANSPORT_MODE_TO_MAPBOX
   from cartodb_services.tools import Coordinate
   from cartodb_services.metrics import QuotaService
   from cartodb_services.tools import Logger,LoggerConfig
@@ -159,7 +159,7 @@ RETURNS SETOF cdb_dataservices_server.isoline AS $$
     else:
       raise Exception('source is NULL')
 
-    profile = MODE_TO_MAPBOX_PROFILE.get(mode)
+    profile = TRANSPORT_MODE_TO_MAPBOX.get(mode)
 
     # -- TODO Support options properly
     isolines = {}
@@ -266,7 +266,7 @@ CREATE OR REPLACE FUNCTION cdb_dataservices_server._cdb_mapbox_isochrones(
 RETURNS SETOF cdb_dataservices_server.isoline AS $$
   import json
   from cartodb_services.mapbox import MapboxMatrixClient, MapboxIsolines
-  from cartodb_services.mapbox.types import MODE_TO_MAPBOX_PROFILE
+  from cartodb_services.mapbox.types import TRANSPORT_MODE_TO_MAPBOX
   from cartodb_services.tools import Coordinate
   from cartodb_services.tools.coordinates import coordinates_to_polygon
   from cartodb_services.metrics import QuotaService
@@ -294,7 +294,7 @@ RETURNS SETOF cdb_dataservices_server.isoline AS $$
     else:
       raise Exception('source is NULL')
 
-    profile = MODE_TO_MAPBOX_PROFILE.get(mode)
+    profile = TRANSPORT_MODE_TO_MAPBOX.get(mode)
 
     resp = mapbox_isolines.calculate_isochrone(origin, data_range, profile)
 

server/lib/python/cartodb_services/cartodb_services/mapbox/geocoder.py

@@ -38,9 +38,13 @@ class MapboxGeocoder(Traceable):
 
     def _parse_geocoder_response(self, response):
         json_response = json.loads(response)
+
+        if json_response:
             feature = json_response[ENTRY_FEATURES][0]
 
             return self._extract_lng_lat_from_feature(feature)
+        else:
+            return []
 
     def _extract_lng_lat_from_feature(self, feature):
         geometry = feature[ENTRY_GEOMETRY]
@@ -62,11 +66,26 @@ class MapboxGeocoder(Traceable):
         if state_province:
             address.append(state_province)
 
+        try:
             response = self._geocoder.forward(address=', '.join(address),
                                               country=country,
                                               limit=1)
 
             if response.status_code == requests.codes.ok:
                 return self._parse_geocoder_response(response.text)
+            elif response.status_code == requests.codes.bad_request:
+                return []
             else:
                 raise ServiceException(response.status_code, response)
+        except requests.Timeout as te:
+            # In case of timeout we want to stop the job because the server
+            # could be down
+            self._logger.error('Timeout connecting to Mapbox geocoding server',
+                               te)
+            raise ServiceException('Error geocoding {0} using Mapbox'.format(
+                searchtext), None)
+        except requests.ConnectionError as ce:
+            # Don't raise the exception to continue with the geocoding job
+            self._logger.error('Error connecting to Mapbox geocoding server',
+                               exception=ce)
+            return []

server/lib/python/cartodb_services/cartodb_services/mapbox/matrix_client.py

@@ -64,9 +64,25 @@ class MapboxMatrixClient(Traceable):
         coords = marshall_coordinates(coordinates)
 
         uri = self._uri(coords, profile)
+
+        try:
             response = requests.get(uri)
 
             if response.status_code == requests.codes.ok:
                 return response.text
+            elif response.status_code == requests.codes.bad_request:
+                return '{}'
             else:
                 raise ServiceException(response.status_code, response)
+        except requests.Timeout as te:
+            # In case of timeout we want to stop the job because the server
+            # could be down
+            self._logger.error('Timeout connecting to Mapbox matrix service',
+                               te)
+            raise ServiceException('Error getting matrix data from Mapbox',
+                                   None)
+        except requests.ConnectionError as ce:
+            # Don't raise the exception to continue with the geocoding job
+            self._logger.error('Error connecting to Mapbox matrix service',
+                               exception=ce)
+            return '{}'

server/lib/python/cartodb_services/cartodb_services/mapbox/routing.py

@@ -63,6 +63,8 @@ class MapboxRouting(Traceable):
 
     def _parse_routing_response(self, response):
         json_response = json.loads(response)
+
+        if json_response:
             route = json_response[ENTRY_ROUTES][0]  # Force the first route
 
             geometry = PolyLine().decode(route[ENTRY_GEOMETRY])
@@ -70,6 +72,8 @@ class MapboxRouting(Traceable):
             duration = route[ENTRY_DURATION]
 
             return MapboxRoutingResponse(geometry, distance, duration)
+        else:
+            return MapboxRoutingResponse(None, None, None)
 
     @qps_retry(qps=1)
     def directions(self, waypoints, profile=DEFAULT_PROFILE):
@@ -79,6 +83,8 @@ class MapboxRouting(Traceable):
         coordinates = marshall_coordinates(waypoints)
 
         uri = self._uri(coordinates, profile)
+
+        try:
             response = requests.get(uri)
 
             if response.status_code == requests.codes.ok:
@@ -87,6 +93,18 @@ class MapboxRouting(Traceable):
                 return MapboxRoutingResponse(None, None, None)
             else:
                 raise ServiceException(response.status_code, response)
+        except requests.Timeout as te:
+            # In case of timeout we want to stop the job because the server
+            # could be down
+            self._logger.error('Timeout connecting to Mapbox routing service',
+                               te)
+            raise ServiceException('Error getting routing data from Mapbox',
+                                   None)
+        except requests.ConnectionError as ce:
+            # Don't raise the exception to continue with the geocoding job
+            self._logger.error('Error connecting to Mapbox routing service',
+                               exception=ce)
+            return MapboxRoutingResponse(None, None, None)
 
 
 class MapboxRoutingResponse:

server/lib/python/cartodb_services/cartodb_services/mapbox/types.py

@@ -1,4 +1,4 @@
-MODE_TO_MAPBOX_PROFILE = {
+TRANSPORT_MODE_TO_MAPBOX = {
     'car': 'driving',
     'walk': 'walking',
 }

server/lib/python/cartodb_services/cartodb_services/metrics/config.py

@@ -166,6 +166,10 @@ class RoutingConfig(ServiceConfig):
     def provider(self):
         return self._routing_provider
 
+    @property
+    def mapzen_provider(self):
+        return self._routing_provider == self.MAPZEN_PROVIDER
+
     @property
     def mapzen_api_key(self):
         return self._mapzen_api_key
@@ -174,6 +178,10 @@ class RoutingConfig(ServiceConfig):
     def mapzen_service_params(self):
         return self._mapzen_service_params
 
+    @property
+    def mapbox_provider(self):
+        return self._routing_provider == self.MAPBOX_PROVIDER
+
     @property
     def mapbox_api_key(self):
         return self._mapbox_api_key