Add check for a NO_RESULT to GeocodeFarm - #240
Conversation
…d of an exception" This reverts commit 7e61e8c.
KostyaEsmukov
left a comment
There was a problem hiding this comment.
May I ask you to add a test for this change? Thanks in advance.
|
@KostyaEsmukov test added in 6d8d023 |
| 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.
|
|
||
| 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.
Issue #239
Added two lines to check for the
NO_RESULTSstatus from webservice and returnNoneinstead of an exception.