Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return a 400 status code for an "out-of-range" get tile request #1503

Merged
merged 3 commits into from
Jan 31, 2024

Conversation

doublebyte1
Copy link
Contributor

@doublebyte1 doublebyte1 commented Jan 17, 2024

Overview

The MVT-elastic backend provider returns a 400 status code in response to a request for a tile out of range (e.g.: outside the limits of the TMS). This would satisfy req 6 A of the OGC API - Tiles standard (requirements class core). However, pygeoapi is transforming this into a 500 error.
This PR introduces a try/ except block on the MVT-elastic class, and makes sure that the Exception that arrives to the API class is appropriated. For that purpose, I reused the existing "ProviderInvalidQueryError", which is close enough in terms of semantics, but I could also have created a new Provider exception.

Related issue / discussion

#1502

Additional information

OGC API - Tiles standard: https://docs.ogc.org/is/20-057/20-057.html

Dependency policy (RFC2)

  • I have ensured that this PR meets RFC2 requirements

Updates to public demo

Contributions and licensing

(as per https://github.com/geopython/pygeoapi/blob/master/CONTRIBUTING.md#contributions-and-licensing)

  • I'd like to contribute [feature X|bugfix Y|docs|something else] to pygeoapi. I confirm that my contributions to pygeoapi will be compatible with the pygeoapi license guidelines at the time of contribution
  • I have already previously agreed to the pygeoapi Contributions and Licensing Guidelines

@doublebyte1 doublebyte1 changed the title Return a 400 status code for an out of range tile request Return a 400 status code for an "out-of-range" get tile request Jan 17, 2024
Copy link
Contributor

@francbartoli francbartoli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@doublebyte1 does the problem apply to the rest of the MVT provider implementations like for example mvt_tippecanoe?

pygeoapi/api.py Outdated Show resolved Hide resolved
pygeoapi/provider/mvt_elastic.py Outdated Show resolved Hide resolved
@doublebyte1
Copy link
Contributor Author

doublebyte1 commented Jan 18, 2024

@doublebyte1 does the problem apply to the rest of the MVT provider implementations like for example mvt_tippecanoe?

@francbartoli MVT-tippecanoe looks for the tile on disk, instead of issuing a request to a server. When it does not find it, it throws a FileNotFound/404 error, which complies with the spec.

https://github.com/geopython/pygeoapi/blame/e1fe05c1c16e40ab8bf6d07540e86d6f78b14172/pygeoapi/provider/mvt_tippecanoe.py#L174-L175

curl -I https://demo.pygeoapi.io/master/collections/lakes/tiles/WorldCRS84Quad/10/2147483647/2147483647?f=mvt

@doublebyte1 doublebyte1 marked this pull request as draft January 18, 2024 10:35
 - if the status code is bellow 500, it throws a ProviderInvalidQueryError (status code 404)
 - otherwise it throws a server error (status code 500)
- the api catching of errors is left as it was before, as the appropriated status code is being sent by the provider.
@doublebyte1 doublebyte1 marked this pull request as ready for review January 18, 2024 10:54
return resp.content
except requests.exceptions.RequestException as e:
LOGGER.debug(e)
if resp.status_code <= 500:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think < 500 is meant here instead of <= 500, otherwise internal server errors would be reported as invalid client errors

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed. Well spotted!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@totycro fixed on my latest commit

…nses with a status of 500 from throwing a client error
@francbartoli
Copy link
Contributor

@doublebyte1 does the problem apply to the rest of the MVT provider implementations like for example mvt_tippecanoe?

@francbartoli MVT-tippecanoe looks for the tile on disk, instead of issuing a request to a server. When it does not find it, it throws a FileNotFound/404 error, which complies with the spec.

https://github.com/geopython/pygeoapi/blame/e1fe05c1c16e40ab8bf6d07540e86d6f78b14172/pygeoapi/provider/mvt_tippecanoe.py#L174-L175

curl -I https://demo.pygeoapi.io/master/collections/lakes/tiles/WorldCRS84Quad/10/2147483647/2147483647?f=mvt

@doublebyte1 if I'm not wrong pygeoapi supported tippecanoe built tiles which might be hosted remotely on s3-like (i.e. Minio, etc) buckets

@doublebyte1 doublebyte1 marked this pull request as draft January 18, 2024 19:01
@doublebyte1
Copy link
Contributor Author

doublebyte1 commented Jan 18, 2024

@doublebyte1 does the problem apply to the rest of the MVT provider implementations like for example mvt_tippecanoe?

@francbartoli MVT-tippecanoe looks for the tile on disk, instead of issuing a request to a server. When it does not find it, it throws a FileNotFound/404 error, which complies with the spec.
https://github.com/geopython/pygeoapi/blame/e1fe05c1c16e40ab8bf6d07540e86d6f78b14172/pygeoapi/provider/mvt_tippecanoe.py#L174-L175
curl -I https://demo.pygeoapi.io/master/collections/lakes/tiles/WorldCRS84Quad/10/2147483647/2147483647?f=mvt

@doublebyte1 if I'm not wrong pygeoapi supported tippecanoe built tiles which might be hosted remotely on s3-like (i.e. Minio, etc) buckets

@francbartoli I addressed this issue on a separate PR: #1507

@doublebyte1 doublebyte1 marked this pull request as ready for review January 22, 2024 10:21
@tomkralidis tomkralidis merged commit de787b0 into geopython:master Jan 31, 2024
4 checks passed
@doublebyte1 doublebyte1 deleted the req6A-tiles branch March 6, 2024 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants