Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Jormun] optim geovelo #2711

Merged
merged 6 commits into from
Mar 20, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 24 additions & 19 deletions source/jormungandr/jormungandr/street_network/geovelo.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@
import logging
import requests as requests
import pybreaker
import json
import ujson

import itertools
import sys
from navitiacommon import response_pb2
from jormungandr import app
from jormungandr.exceptions import TechnicalError, InvalidArguments, UnableToParse
Expand Down Expand Up @@ -89,7 +92,7 @@ def status(self):
@classmethod
def _pt_object_summary_isochrone(cls, pt_object):
coord = get_pt_object_coord(pt_object)
return [coord.lat, coord.lon, getattr(pt_object, 'uri', None)]
return [coord.lat, coord.lon, None]

@classmethod
def _make_request_arguments_bike_details(cls, bike_speed_mps):
Expand Down Expand Up @@ -131,7 +134,11 @@ def _call_geovelo(self, url, method=requests.post, data=None):
url,
timeout=self.timeout,
data=data,
headers={'content-type': 'application/json', 'Api-Key': self.api_key},
headers={
'content-type': 'application/json',
'Api-Key': self.api_key,
'Accept-Encoding': 'gzip, br',
},
)
except pybreaker.CircuitBreakerError as e:
logging.getLogger(__name__).error('Geovelo routing service dead (error: {})'.format(e))
Expand All @@ -157,14 +164,12 @@ def _get_matrix(cls, json_response):
if json_response[0] != ["start_reference", "end_reference", "duration"]:
logging.getLogger(__name__).error('Geovelo parsing error. Response: {}'.format(json_response))
raise UnableToParse('Geovelo parsing error. Response: {}'.format(json_response))
for e in json_response[1:]:
routing = row.routing_response.add()
if e[2]:
routing.duration = e[2]
routing.routing_status = response_pb2.reached
else:
routing.duration = -1
routing.routing_status = response_pb2.unknown

add_ = row.routing_response.add
for e in itertools.islice(json_response, 1, sys.maxint):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use None for stop?
And are we sure that geovelo keep the request order? And by sure I mean does is it documented?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since in the old code, we didn't check the order... I assumed that the order was kept

duration, routing_status = (e[2], response_pb2.reached) if e[2] else (-1, response_pb2.unknown)
add_(duration=duration, routing_status=routing_status)

return sn_routing_matrix

@classmethod
Expand Down Expand Up @@ -198,10 +203,10 @@ def get_street_network_routing_matrix(

data = self._make_request_arguments_isochrone(origins, destinations, request['bike_speed'])
r = self._call_geovelo(
'{}/{}'.format(self.service_url, 'api/v2/routes_m2m'), requests.post, json.dumps(data)
'{}/{}'.format(self.service_url, 'api/v2/routes_m2m'), requests.post, ujson.dumps(data)
)
self._check_response(r)
resp_json = r.json()
resp_json = ujson.loads(r.text)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/kennethreitz/requests/issues/1595
It will be nice to have it for everyone :)

Copy link
Member Author

@xlqian xlqian Mar 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually, i was looking forward to being able to specify the json module in .json(), like r.json(json_module=ujson)...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mypy doesn't like requests.models.json = ujson


if len(resp_json) - 1 != len(origins) * len(destinations):
logging.getLogger(__name__).error('Geovelo nb response != nb requested')
Expand Down Expand Up @@ -283,7 +288,9 @@ def _get_response(cls, json_response, pt_object_origin, pt_object_destination, f

speed = section.length / section.duration if section.duration != 0 else 0

for geovelo_instruction in geovelo_section['details']['instructions'][1:]:
for geovelo_instruction in itertools.islice(
geovelo_section['details']['instructions'], 1, sys.maxint
):
path_item = section.street_network.path_items.add()
path_item.name = geovelo_instruction[1]
path_item.length = geovelo_instruction[2]
Expand All @@ -292,9 +299,7 @@ def _get_response(cls, json_response, pt_object_origin, pt_object_destination, f

shape = decode_polyline(geovelo_resp['sections'][0]['geometry'])
for sh in shape:
coord = section.street_network.coordinates.add()
coord.lon = sh[0]
coord.lat = sh[1]
section.street_network.coordinates.add(lon=sh[0], lat=sh[1])

return resp

Expand All @@ -320,10 +325,10 @@ def _direct_path(
'objects_as_ids=true&',
),
requests.post,
json.dumps(data),
ujson.dumps(data),
)
self._check_response(r)
resp_json = r.json()
resp_json = ujson.loads(r.text)

if len(resp_json) != 1:
logging.getLogger(__name__).error('Geovelo nb response != nb requested')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
from .streetnetwork_test_utils import make_pt_object
from jormungandr.utils import str_to_time_stamp, PeriodExtremity
import requests_mock
import json
import ujson

MOCKED_REQUEST = {'walking_speed': 1, 'bike_speed': 3.33}

Expand Down Expand Up @@ -208,7 +208,7 @@ def pt_object_summary_test():
summary = Geovelo._pt_object_summary_isochrone(
make_pt_object(type_pb2.ADDRESS, lon=1.12, lat=13.15, uri='toto')
)
assert summary == [13.15, 1.12, 'toto']
assert summary == [13.15, 1.12, None]


def make_data_test():
Expand All @@ -218,9 +218,9 @@ def make_data_test():
make_pt_object(type_pb2.ADDRESS, lon=4, lat=48.4, uri='refEnd2'),
]
data = Geovelo._make_request_arguments_isochrone(origins, destinations)
assert json.loads(json.dumps(data)) == json.loads(
assert ujson.loads(ujson.dumps(data)) == ujson.loads(
'''{
"starts": [[48.2, 2.0, "refStart1"]], "ends": [[48.3, 3.0, "refEnd1"], [48.4, 4.0, "refEnd2"]],
"starts": [[48.2, 2.0, null]], "ends": [[48.3, 3.0, null], [48.4, 4.0, null]],
"transportMode": "BIKE",
"bikeDetails": {"profile": "MEDIAN", "averageSpeed": 12, "bikeType": "TRADITIONAL"}}'''
)
Expand Down Expand Up @@ -378,13 +378,13 @@ def make_request_arguments_bike_details_test():
instance = MagicMock()
geovelo = Geovelo(instance=instance, service_url='http://bob.com')
data = geovelo._make_request_arguments_bike_details(bike_speed_mps=3.33)
assert json.loads(json.dumps(data)) == json.loads(
assert ujson.loads(ujson.dumps(data)) == ujson.loads(
'''{"profile": "MEDIAN", "averageSpeed": 12,
"bikeType": "TRADITIONAL"}'''
)

data = geovelo._make_request_arguments_bike_details(bike_speed_mps=4.1)
assert json.loads(json.dumps(data)) == json.loads(
assert ujson.loads(ujson.dumps(data)) == ujson.loads(
'''{"profile": "MEDIAN", "averageSpeed": 15,
"bikeType": "TRADITIONAL"}'''
)