Postcode logging #620
Postcode logging #620
Conversation
GeoWill
force-pushed
the
postcode-logging
branch
3 times, most recently
from
8e64fde to
f0805ca
Compare
GeoWill
force-pushed
the
postcode-logging
branch
from
f0805ca to
7715350
Compare
| postcode_location = result.get("postcode_location", {}) | ||
| properties = postcode_location.get("properties", {}) | ||
| postcode = properties.get("postcode", None) |
There was a problem hiding this comment.
This feels very defensive, but I figure we don't want to fall over here because there wasn't a postcode_location or properties object.
There was a problem hiding this comment.
Same comment from the EC API. We don't want to log None postcodes?
There was a problem hiding this comment.
Yep, I wasn't sure which way to go. I think we shouldn't end up logging None postcodes because we should always have a postcode from WDIV. If this broke then I was thinking that logging them was a good way to be able to look back and see how long the problem was. I guess it does just pollute the logs table though. Happy to put a check in and not log if postcode is None
There was a problem hiding this comment.
Yeah, I think it would be better to not log here, but to raise an error in Sentry. We still get to see when things are going wrong, but don't persist this to the logs
If WDIV returns a list of addresses don't log. We will log the result returned if there is a follow up request on the address endpoint.
GeoWill
force-pushed
the
postcode-logging
branch
from
7715350 to
d276369
Compare
There was a problem hiding this comment.
These wdiv_address.json files are included because we were using the same fixures to mock responses from the WDIV postcode endpoint and the WDIV address endpoint. The only difference is that the address endpoint always includes a single address object in the addresses array. This was how I was going to get the postcode, before it was added to the postcode_location geojson. However I left these files here since they make it a bit more explicit that the endpoints aren't exactly the same.
GeoWill
force-pushed
the
postcode-logging
branch
from
97a9d29 to
e7c7606
Compare
GeoWill
force-pushed
the
postcode-logging
branch
from
e7c7606 to
4e9d63e
Compare
2 participants
Rebased on #621
Wait till doc wording is figured out before merging.