Closed trevorskaggs closed 7 years ago
See #10 for my proposal
@AlexMountain @trevorskaggs This comment will be in flux while I document my findings. Hopefully you will find it useful once I'm done poking and prodding.
Also, note that even when I say we should update the docs to match what's been implemented, I'm generally making these changes in the Swagger branch that I'm writing. So, no need to continue update the rst files. As long as everything I'm finding seems to be responding the way you'd expect.
Tested on TE with my API key substituted for {{source}}
. The WKT polygon
POLYGON ((62.1873999999999967 34.3468000000000018, 62.1873999999999967 34.3451999999999984, 62.1901000000000010 34.3451999999999984, 62.1901000000000010 34.3468000000000018, 62.1873999999999967 34.3468000000000018))
is substituded for {{geom}}
. Various :pk
values were selected from my own resources.
GET /aoi?source={{source}}&geom={{geom}}
aoi
objects. It is actually an object aoi_list
, which is an array of aoi
objects. This is probably fine, just need to update docs.GET /aoi/:pk?source={{source}}
aoi
object. The documentation indicates that these will be at the root level, alongside pointcloud_intersects
, etc. To me, it seems redundant to nest this information.GET /aoi/edit/:pk?source={{source}}¬es=This%20is%20a%20note
aoi
object. The documentation indicates that these will be at the root level, alongside pointcloud_intersects
, etc. To me, it seems redundant to nest this information.GET /aoi/delete/:pk?source={{source}}
{}
. No harm done necessarily, but if the API states that there will never be any information here, why bother returning anything but the 200 status.GET /aoi/add?source={{source}}&geom={{geom}}&name=v2test&subscribe=True
aoi
object. The documentation indicates that these will be at the root level, alongside pointcloud_intersects
, etc. To me, it seems redundant to nest this information.collect_at
(in both pointcloud_intersects
and raster_intersects
) while the actual field is collected_at
. I like the latter, just need to update docs.GET /aoi/:pk/generate/pointcloud?source={{source}}&products=:pk&name=sonny
pk
is provided. Things blow up. Several expectations were not met. First, I think that the invalid pk
(or pks) should probably just be skipped. Second, if you are going to return an error that is indicating that I made a bad request, it should be a 400 status with a JSON-formatted error message per the docs (not a 200 status with an HTML response). Something like:{
"error": "Please only provide point cloud product pks. I don't know what to do with rasters."
}
GET /aoi/:pk/generate/raster
pk
is provided. Things blow up. Several expectations were not met. First, I think that the invalid pk
(or pks) should probably just be skipped. Second, if you are going to return an error that is indicating that I made a bad request, it should be a 400 status with a JSON-formatted error message per the docs (not a 200 status with an HTML response). Something like:{
"error": "Please only provide raster product pks. I don't know what to do with point clouds."
}
GET /export/:pk?source={{source}}
export
object. The documentation indicates that these will be at the root level, alongside exportfiles
, etc. To me, it seems redundant to nest this information.GET /export/edit/:pk?source={{source}}¬es=This%20is%20another%20note
GET /export/delete/:pk?source={{source}}
GET /export/:pk
on a deleted export and get information about it. Maybe that's by design. Probably just need to document it accordingly.GET /geoname?source={{source}}&geom={{geom}}
GET /product/:pk?source={{source}}
(pointcloud :pk
)
collect_at
while the actual field is collected_at
. I like the latter, just need to update docs.GET /product/:pk?source={{source}}
(raster :pk
)
collect_at
while the actual field is collected_at
. I like the latter, just need to update docs.GET /task/:task_id?source={{source}}
/api/v2/aoi/1959
returns status code 200 OK
with HTML body indicating a server error. This shouldn't fail, as this particular pk
exists, but if it does, it should return a different status code along with the JSON error object. For example, if user provides invalid pk
, status should be 400
with a message that pk does not exist
or similar./api/v2/aoi/add/?source=toasted_filament&name=kleen&geom=POLYGON%20((62.1873999999999967%2034.3468000000000018,%2062.1873999999999967%2034.3451999999999984,%2062.1901000000000010%2034.3451999999999984,%2062.1901000000000010%2034.3468000000000018,%2062.1873999999999967%2034.3468000000000018))&subscribe=false
returns status code 200 OK
with HTML body indicating a server error. Unsure of what could make this fail (perhaps a bad geometry?), but when it does, it should return a different status code along with the JSON error object. For example, a status 400
with a message that the provided geometry was invalid./api/v2/aoi/edit/2651?source=toasted_filament&name=NewName¬es=NewNotes&geom=POLYGON%20((62.1873999999999967%2034.3468000000000018,%2062.1873999999999967%2034.3451999999999984,%2062.1901000000000010%2034.3451999999999984,%2062.1901000000000010%2034.3468000000000018,%2062.1873999999999967%2034.3468000000000018))
returns status code 200 OK with HTML body indicating a server error. Unsure of what could make this fail (perhaps a bad geometry?), but when it does, it should return a different status code along with the JSON error object. For example, a status 400
with a message that the provided geometry was invalid.When generating a raster export of mixed datatype, does it really make sense to return the different datatypes in a comma-separated string? There is no mapping between this and the exportfiles, but it seems like useful information (e.g., QTM loads elevation models differently than imagery). I think that datatype maybe should be returned with each exportfile.
@AlexMountain Tell me if you think this is still doable for v2.
In short, we need better error handling. The docs state that we get status code 200
for successful API calls, 400
for bad requests (client-side), and 500
for server-side issues.
But this rule is generally not followed. For example, you do provide an error message when source
is missing from a request, but the status is still 200
. It should be 400
. In other places (e.g., pk
other than number, pk
out of range), the error message is still not used, and we get the status of 200
along with HTML in the response body.
There are almost certainly others - these are just the ones I tested. Basically any time you want to return that HTML with the title
@AlexMountain I think we can merge this now. Don't you think?
Add collection information to API AOI detail call: