Esri / arcgis-osm-editor

ArcGIS Editor for OpenStreetMap is a toolset for GIS users to access and contribute to OpenStreetMap through their Desktop or Server environment.
Apache License 2.0
395 stars 129 forks source link

Load OSM File tool and OSM File Loader (Load only) should warn for incomplete XML #152

Closed mboeringa closed 7 years ago

mboeringa commented 8 years ago

Hi @ThomasEmge,

As an extension of #151 , it is probably also wise to enhance both the Load OSM File tool and OSM File Loader (Load only) with the capability to detect the same query time out remark XML element in an osm XML file as created by the XAPI servers. The tools should preferably do this test at the very beginning of the load process, and fail with a warning of an invalid osm XML file, as I don't see any use for processing incomplete osm XML files.

Of course, the warning should only be based on a truly detected query time out remark XML element in the osm XML file, osm XML files based on custom overpass API queries may also lack data, but are valid and should be processed (of course, in the latter case, they don't usually contain a query time out remark element, unless there was a true failure as well).

ThomasEmge commented 8 years ago

As you mentioned it will be hard to judge completeness of the download. What I can do is report the element itself but then the user still needs to figure out what to do with this information.

mboeringa commented 8 years ago

I am certainly not suggesting to check for "completeness" of the osm XML file in the sense of containing all objects in a possibly limited extent, that is indeed impossible. As I wrote, an Overpass (Turbo) API query may - and usually does - result in the deliberate extraction of only a subset of the OSM data, which may even span the entire globe.

"but then the user still needs to figure out what to do with this information."

Yes, if you do detect the specific "remark" element referencing a query time out, it is up to the user to decide what to do with it.

In a sense, it might be best if this test was - additionally - implemented at the toolvalidation level, and not just runtime, and set as "Warning" error level message, instead of "Error", because in that case, the message already appears before running the tool when selecting the OSM file, and the user can decide what to do with it before he hits the "OK"

I am just not sure what the consequence is if you attempt to extract such an XML element from a potentially dozens of GB big OSM file in toolvalidation code... Does the size matter if a proper XPath statement or such is used?

If that is not feasible, doing it at the runtime of the tool, and showing a warning at the end of the process explaining the situation and detected query time out element, is probably the way to go. At least, the user should then be informed of the situation.

mboeringa commented 7 years ago

Seems taken care of in your https://github.com/Esri/arcgis-osm-editor/commit/480a763615d738cfdda34a940c0c051ba8e16371 commit, so I will close it for now.