Add check for a NO_RESULT to GeocodeFarm #240
Conversation
…d of an exception" This reverts commit 7e61e8c.
|
@KostyaEsmukov test added in 6d8d023 |
| @@ -159,6 +161,7 @@ def _check_for_api_errors(geocoding_results): | |||
| in the api response. | |||
| """ | |||
| status_result = geocoding_results.get("STATUS", {}) | |||
| if "NO_RESULTS" in status_result.get("status", ""): return | |||
There was a problem hiding this comment.
Is there any reason to have this check here? If there's none, I suggest to remove this line.
As far as I can see the following two lines shouldn't misbehave even when the status contains NO_RESULTS.
There was a problem hiding this comment.
Sorry, if I'm being honest this was long enough ago that I forgot my rational for the double lines... I'll figure that out and cleanup.
There was a problem hiding this comment.
@KostyaEsmukov I double checked, both checks are necessary.
test/geocoders/geocodefarm.py
Outdated
| @@ -96,6 +96,35 @@ def mock_call_geocoder(*args, **kwargs): | |||
| with self.assertRaises(exc.GeocoderQuotaExceeded): | |||
| self.geocoder.geocode('435 north michigan ave, chicago il 60611') | |||
|
|
|||
| def test_no_results(self): | |||
| """ | |||
| GeocodeFarm unhandled error | |||
There was a problem hiding this comment.
This is clearly a copy-paste from another test. You can remove this comment completely (instead of updating it), because after switch to pytest (#266) they all will be removed anyway.
There was a problem hiding this comment.
Yes, I wanted to emulate existing tests instead of reinventing unnecessarily... I was sloppy and missed that comment. Will remove.
There was a problem hiding this comment.
@KostyaEsmukov removed that comment
Issue #239
Added two lines to check for the
NO_RESULTSstatus from webservice and returnNoneinstead of an exception.