geopython / GeoHealthCheck

Service Status and QoS Checker for OGC Web Services
https://geohealthcheck.org
MIT License
86 stars 69 forks source link

KeyError 'tilejson' on vector tiles probe #407

Open pvgenuchten opened 3 years ago

pvgenuchten commented 3 years ago

Describe the bug we host a vector tile service at https://tiles.isric.org based on TRex, when i run the tilejson through geohealthcheck, i get this error:

      "class": "GeoHealthCheck.plugins.probe.mapbox.TileJSON",
      "message": "Perform_request Err: KeyError 'tilejson'",
      "name": "TileJSON",
      "probe_id": 1,
      "response_time": "0.78631",
      "success": false

I wonder if this is a problem on our configuration or on the probe, some guidance would be helpfull.

pvgenuchten commented 3 years ago

The problem seems at https://github.com/geopython/GeoHealthCheck/blob/a25e87125eec4eea57ab26999d67f89dbf5f0872/GeoHealthCheck/plugins/probe/mapbox.py#L42

Code seems to assume a key 'tilejson' to be available, which is not the case in our implementation. Which indeed is not according to specification. But would be good to test this aspect explicitly and flag if the required parameter is missing.

justb4 commented 3 years ago

Good one @pvgenuchten ! Yes the tilejson field is mandatory, but maybe not in all versions. The TileJSON Probe is brand new: merged yesterday via PR #405. @jochemthart1 and I already found some problems. Also I am not too familiar with the spec, it seems less formal than OGC specs, especially for the metadata like bounds and center (both optional). Maybe you have suggestions for us.

What is unclear to me:

Offcourse you can always add a regular URLOpen Probe for a single specific tile and possibly even check for a keyword or content-type, but we would rather have a real TileJSON probe.

jochemthart1 commented 3 years ago

@pvgenuchten The check for tilejson startswith 2 was there, because version 2 was not supported. However, I didn't realize that tilejson is not mandatory in all versions.

I have now removed the version requirement and I am trying to fix the errors that arise.

With this change, I still run into an issue with this specific url: tilejson. Zoom levels 1-13 are be alright, but 14 causes too many 500 error responses. Request urls are for example:

As you can see, zoom 14 won't return anything.

Is this something in your configuration or is this something with the requested url?

pvgenuchten commented 3 years ago

@jochemthart1 thanx for checking the fact that level 14 is not available relates to the fact that tilejson.json indicates that max-zoomlevel is 12....

jochemthart1 commented 3 years ago

@jochemthart1 thanx for checking the fact that level 14 is not available relates to the fact that tilejson.json indicates that max-zoomlevel is 12....

Ah, the available zoom-levels are taken from the minzoom and maxzoom of the root, which is set to 0-15 in the tilejson.json.

pvgenuchten commented 3 years ago

Oh let me check that, seems we have a contradiction in our tile json file?

justb4 commented 3 years ago

See also my latest review on PR #409, mainly considering potential KeyErrors. Another safe way to access Python dict is using get() with a default value e.g.:

center_coords = tile_info.get('center', None)
if not center_coords: