search

grnet / djnro

DjNRO hits the decks of eduroam database management
http://djnro.grnet.gr/
Other
10 stars 21 forks source link

Fix: Closest point API: ignore individual point parsing errors #82

Closed vladimir-mencl-eresearch closed 3 years ago

vladimir-mencl-eresearch commented 3 years ago

Hi @zmousm ,

I hope you are doing well.

Last week, we ran into issues where the upstream KML file had unparsable data in latitude/longitude fields (coordinates accidentally expressed as 'lat': "57°0'56.", 'lng': "9°58'36." - with unicode degree character!

DjNRO started emailing us stack-trace error messages for each hit to the closest point API - and users were getting 500 Internal Server error.

This PR is a work-around that catches ValueError within the loop, just skipping invalid entries.

I raised the issue with the eduroam OT - they were not able to find where that error came from, and it disappeared in the meantime. I still have a copy of the invalid data ... but while it's not possible to track how it got into the KML file, it seems to be reasonable to have a safeguard against future invalid data.

Does this look OK to you to merge?

Cheers, Vlad

zmousm commented 3 years ago

Hello @vladimir-mencl-eresearch

I saw an XML parsing error (perhaps related) around the same time:

xml.etree.ElementTree.ParseError: not well-formed (invalid token): line 94808, column 64

Ideally any such issues should be caught through input (XML schema) validation, but I agree such a safeguard is reasonable as well, in the short term.

Please see the inline comment.

vladimir-mencl-eresearch commented 3 years ago

Now that the exception catching block is limited to float casting statements, does this look OK to merge? Cheers, Vlad