diff --git a/server/lib/python/cartodb_services/cartodb_services/geocoder.py b/server/lib/python/cartodb_services/cartodb_services/geocoder.py index 03ad014..66dffc7 100644 --- a/server/lib/python/cartodb_services/cartodb_services/geocoder.py +++ b/server/lib/python/cartodb_services/cartodb_services/geocoder.py @@ -55,19 +55,25 @@ def run_street_point_geocoder(plpy, GD, geocoder, service_manager, username, org if not geocode_results == EMPTY_BATCH_RESPONSE: for result in geocode_results: metadata = result[2] if len(result) > 2 else {} - - if metadata.get('error', None): + try: + if metadata.get('error', None): + results.append([result[0], None, json.dumps(metadata)]) + a_failed_one = result + failed_count += 1 + elif result[1] and len(result[1]) == 2: + plan = plpy.prepare("SELECT ST_SetSRID(ST_MakePoint($1, $2), 4326) as the_geom; ", ["double precision", "double precision"]) + point = plpy.execute(plan, result[1], 1)[0] + results.append([result[0], point['the_geom'], json.dumps(metadata)]) + success_count += 1 + else: + results.append([result[0], None, json.dumps(metadata)]) + empty_count += 1 + except Exception as e: + import sys + logger.error("Error processing geocode", sys.exc_info(), data={"username": username, "orgname": orgname}) + metadata['processing_error'] = 'Error: {}'.format(e.message) results.append([result[0], None, json.dumps(metadata)]) - a_failed_one = result failed_count += 1 - elif result[1] and len(result[1]) == 2: - plan = plpy.prepare("SELECT ST_SetSRID(ST_MakePoint($1, $2), 4326) as the_geom; ", ["double precision", "double precision"]) - point = plpy.execute(plan, result[1], 1)[0] - results.append([result[0], point['the_geom'], json.dumps(metadata)]) - success_count += 1 - else: - results.append([result[0], None, json.dumps(metadata)]) - empty_count += 1 missing_count = len(searches) - success_count - failed_count - empty_count diff --git a/server/lib/python/cartodb_services/test/test_cartodb_services_geocoder.py b/server/lib/python/cartodb_services/test/test_cartodb_services_geocoder.py index a58d5c2..ad36f72 100644 --- a/server/lib/python/cartodb_services/test/test_cartodb_services_geocoder.py +++ b/server/lib/python/cartodb_services/test/test_cartodb_services_geocoder.py @@ -23,7 +23,15 @@ SEARCH_FIXTURES = { 'error': [ StreetGeocoderSearch(id=200, address=None, city=None, state=None, country=None), - ] + ], + 'broken_middle': [ + StreetGeocoderSearch(id=301, address='Paseo Zorrilla 1, Valladolid', + city=None, state=None, country=None), + StreetGeocoderSearch(id=302, address='Marsopolis', + city=None, state=None, country=None), + StreetGeocoderSearch(id=303, address='Paseo Zorrilla 2, Valladolid', + city=None, state=None, country=None) + ], } BULK_RESULTS_FIXTURES = { @@ -36,6 +44,11 @@ BULK_RESULTS_FIXTURES = { ], 'error': [ (200, [], {'error': 'Something wrong happened'}) + ], + 'broken_middle': [ + (301, [0, 0], {}), + (302, ['a', 'b'], {}), + (303, [0, 0], {}), ] } @@ -49,6 +62,11 @@ EXPECTED_RESULTS_FIXTURES = { ], 'error': [ [200, None, '{"error": "Something wrong happened"}'] + ], + 'broken_middle': [ + [301, [0, 0], '{}'], + [302, None, '{"processing_error": "Error: NO!"}'], + [303, [0, 0], '{}'], ] } @@ -87,7 +105,6 @@ class TestRunStreetPointGeocoder(TestCase): def test_count_increment_total_and_failed_service_use_on_error(self): self.service_manager_mock.assert_within_limits = \ Mock(side_effect=Exception('Fail!')) - searches = [] with(self.assertRaises(BaseException)): @@ -96,20 +113,17 @@ class TestRunStreetPointGeocoder(TestCase): self.quota_service_mock.increment_total_service_use. \ assert_called_once_with(len(searches)) - self.quota_service_mock.increment_failed_service_use. \ assert_called_once_with(len(searches)) def test_count_increment_failed_service_use_on_quota_error(self): self.service_manager_mock.assert_within_limits = \ Mock(side_effect=QuotaExceededException()) - searches = SEARCH_FIXTURES['two'] result = self._run_geocoder(service_manager=self.service_manager_mock, searches=searches) assert_equal(result, []) - self.quota_service_mock.increment_failed_service_use. \ assert_called_once_with(len(searches)) @@ -124,10 +138,10 @@ class TestRunStreetPointGeocoder(TestCase): [2, [0, 0], '{}'], ] self.geocoder_mock.bulk_geocode = MagicMock(return_value=results) + result = self._run_geocoder(geocoder=self.geocoder_mock, searches=searches) assert_equal(result, expected_results) - self.quota_service_mock.increment_success_service_use. \ assert_called_once_with(len(results)) @@ -135,11 +149,11 @@ class TestRunStreetPointGeocoder(TestCase): searches = SEARCH_FIXTURES['two'] results = [] self.geocoder_mock.bulk_geocode = MagicMock(return_value=results) + result = self._run_geocoder(geocoder=self.geocoder_mock, searches=searches) assert_equal(result, results) - self.quota_service_mock.increment_empty_service_use. \ assert_called_once_with(len(searches)) @@ -147,11 +161,11 @@ class TestRunStreetPointGeocoder(TestCase): searches = SEARCH_FIXTURES['two'] + SEARCH_FIXTURES['wrong'] bulk_results = BULK_RESULTS_FIXTURES['two'] + BULK_RESULTS_FIXTURES['wrong'] self.geocoder_mock.bulk_geocode = MagicMock(return_value=bulk_results) + result = self._run_geocoder(geocoder=self.geocoder_mock, searches=searches) assert_equal(result, EXPECTED_RESULTS_FIXTURES['two'] + EXPECTED_RESULTS_FIXTURES['wrong']) - self.quota_service_mock.increment_success_service_use. \ assert_called_once_with(len(SEARCH_FIXTURES['two'])) self.quota_service_mock.increment_empty_service_use. \ @@ -161,14 +175,37 @@ class TestRunStreetPointGeocoder(TestCase): searches = SEARCH_FIXTURES['two'] + SEARCH_FIXTURES['error'] bulk_results = BULK_RESULTS_FIXTURES['two'] + BULK_RESULTS_FIXTURES['error'] self.geocoder_mock.bulk_geocode = MagicMock(return_value=bulk_results) + result = self._run_geocoder(geocoder=self.geocoder_mock, searches=searches) assert_equal(result, EXPECTED_RESULTS_FIXTURES['two'] + EXPECTED_RESULTS_FIXTURES['error']) - self.quota_service_mock.increment_success_service_use. \ assert_called_once_with(len(SEARCH_FIXTURES['two'])) self.quota_service_mock.increment_failed_service_use. \ assert_called_once_with(len(SEARCH_FIXTURES['error'])) + def test_controlled_failure_on_query_break(self): + searches = SEARCH_FIXTURES['broken_middle'] + bulk_results = BULK_RESULTS_FIXTURES['broken_middle'] + self.geocoder_mock.bulk_geocode = MagicMock(return_value=bulk_results) + def break_on_302(*args): + if len(args) == 3: + plan, values, limit = args + if values[0] == 'a': + raise Exception('NO!') + + return [{'the_geom': [0,0]}] + self.plpy_mock.execute = break_on_302 + + result = self._run_geocoder(geocoder=self.geocoder_mock, + searches=searches) + + assert_equal(result, EXPECTED_RESULTS_FIXTURES['broken_middle']) + self.quota_service_mock.increment_success_service_use. \ + assert_called_once_with(2) + self.quota_service_mock.increment_failed_service_use. \ + assert_called_once_with(1) + +