BritishGeologicalSurvey / pyagsapi

pyagsapi - An AGS Utilities API with AGS v4.x Schema Validation & Converter for .ags<-->.xslx files
https://britishgeologicalsurvey.github.io/pyagsapi/
GNU Lesser General Public License v3.0
12 stars 2 forks source link

Export of boreholes by IDs and polygon #118

Closed ximenesuk closed 1 year ago

ximenesuk commented 1 year ago

This PR closes #99 providing two endpoints:

All the tests should all pass locally. A number of integration tests are expected to expected to fail using github actions while the upstream endpoint is not externally available.

Using an internally deployed server check that ags_export:

And check that ags_export_by_polygon:

See the tests for some example cases.

KoalaGeo commented 1 year ago

@volcan01010 if your happy I'll make the front end changes to use these endpoints and release

KoalaGeo commented 1 year ago

@volcan01010 & @ximenesuk I've added the links to the ags_export endpoint in the leaflet map popups.

I'm holding off adding the polygon select functionality for now, but having the endpoint available is great!

Once reviewed I'll get it released and deployed.

ximenesuk commented 1 year ago

@volcan01010 & @ximenesuk I've added the links to the ags_export endpoint in the leaflet map popups.

I'm holding off adding the polygon select functionality for now, but having the endpoint available is great!

Once reviewed I'll get it released and deployed.

John is off this week but hopefully he can review it fairly quickly once he's back. As an aside the API allows the download of multiple AGS files in one call, though this was more for internal reuse reasons and need not be exposed via the seb service.

volcan01010 commented 1 year ago

I wonder if we should use a more generic 400 Bad Request error for when you request a Polygon with more than 10 boreholes. To me, the 404 suggests that we have gone to a URL that doesn't exist, rather than given some bad data.

Here is what the HTTP cats say:

The returned JSON says "Bad request" in a few locations:

{
  "msg": "Not found",
  "type": "bad request",
  "self": "http://localhost:8000/ags_export_by_polygon/?polygon=POLYGON%28%28-4.5%2056%2C-4%2056%2C-4%2055.5%2C-4.5%2055.5%2C-4.5%2056%29%29&count_only=false",
  "errors": [
    {
      "error": "Bad request",
      "propName": null,
      "desc": "More than 10 boreholes (2211) found in the given polygon. Please try with a smaller polygon"
    }
  ]
}
ximenesuk commented 1 year ago

I wonder if we should use a more generic 400 Bad Request error for when you request a Polygon with more than 10 boreholes. To me, the 404 suggests that we have gone to a URL that doesn't exist, rather than given some bad data.

I'm more than happy to go to 400. You are right that 404 doesn't really express the error here and 422 didn't seem right as the polygon is valid.