-
Notifications
You must be signed in to change notification settings - Fork 31
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
Add MapShed Endpoint to Geoprocessing API #3480
Conversation
This very basic endpoint can take a GeoJSON shape or a WKAoI id and starts a MapShed job, which can be queried using the returned job id. This endpoint currently does not support layer overrides. It is also missing API documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some comments.
@@ -178,3 +178,61 @@ | |||
}, | |||
required=['location'], | |||
) | |||
|
|||
nlcd_override_allowed_values = '", "'.join([ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is currently only used for generating the documentation. This should also be used for validating the input.
type=TYPE_OBJECT, | ||
properties={ | ||
'area_of_interest': MULTIPOLYGON, | ||
'wkaoi': Schema( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use a distinct definition here from this one, because that is for a query parameter, and here it is a JSON parameter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice clarification!
@@ -36,5 +36,7 @@ | |||
re_path(r'jobs/' + uuid_regex, get_job, name='get_job'), | |||
re_path(r'modeling/worksheet/$', views.start_modeling_worksheet, | |||
name='start_modeling_worksheet'), | |||
re_path(r'modeling/gwlf-e/prepare/$', views.start_modeling_gwlfe_prepare, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use the hyphenated form gwlf-e
in the URL as that is standardized. See the repository https://github.com/WikiWatershed/gwlf-e and the pypi package https://pypi.org/project/gwlf-e/
from apps.modeling.serializers import AoiSerializer | ||
from apps.modeling.views import _parse_input as _parse_modeling_input |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Importing from modeling.views
is a little messy here. I expect that this will be removed once everything is moved to geoprocessing_api
.
2c75eb7
to
9547c49
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new endpoint works very well! I like your explanation about how this API link is chosen and it does make sense to have it as /modeling/gwlf-e/prepare/
instead of /mapshed/
Is the parameter model description automatically generated? I found it slightly confusing since both the "Area of Interest" and the "Layer Overrides" are big and bold but wkaoi
is relatively small. The description you wrote definitely helped to clarify the parameters.
Thanks for noting that Emily. I added a title to the WKAOI Schema definition to see if that would help in ea87fd1, but unfortunately it did not: I think this is because both Area of Interest and Layer Overrides are complex objects, and can be expanded or collapsed using that big title, but since WKAoI is a simple string it doesn't get the same treatment. It may be worthwhile to upgrade to a more recent version of our Swagger generating repo https://github.com/axnsan12/drf-yasg. I'll make a card for that. |
Previously, all endpoints took a GeoJSON as the POST body, and any additional parameters, e.g. layer name or WKAoI, were path or query parameters. However, for modeling, the requests are much more complex. They may specify multiple layer overrides, HUC IDs, and other modifications. To allow for this, we create a new MODELING_REQUEST schema, which mimics the format used in the internal API, to allow for such complex requests. The Analyze endpoints will continue to use their simple forms. This creates a disparity in the payloads / expected formats between Analyze and Modeling. However, since there is only one existing modeling endpoint (/modeling/worksheet/) and this is used internally, making this switch will be minimally disruptive.
Although all the layers used in MMW can be overridden, only the __LAND__ and __STREAMS__ layers have viable alternatives. Thus, we only document support for those two here. When alternatives are added for other layer types in the future, this documentation should be expanded to include them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is working well for me! I tested with a variety of combinations of aois/wkaois and land overrides as well as no land overrides and everything returned results in the expected format.
Documentation looks good, the only thing I have to add to Emily's review is one question (inline comment) about the last line in the gwlf-e endpoint description
GWLF-E is a more recognizable name for the general public than MapShed. Also, grouping the two endpoints under one path will improve the semantic linking of the two. The other endpoint will be called gwlf-e/run.
ea87fd1
to
ee5990d
Compare
Thanks for all reviews!! Will merge once green. |
Overview
Adds a new endpoint in the geoprocessing API to start a MapShed / GWLF-E Preparation job that takes an area of interest or a WKAoI, a set of optional layer overrides, and generates an input for GWLF-E, which will be consumed by the endpoint made for #3473.
This does not remove the existing internal API, which will continue to be used until the front-end switches to using the new APIs in #3475.
While I initially named the endpoint
/modeling/mapshed
, I ended up going with/modeling/gwlf-e/prepare/
. This is because:/modeling/gwlf-e/run/
will pair the two endpoints in a semantic manner thatmapshed
would not haveAlso, this endpoint takes a more complex payload, described in
MODELING_INPUT
, than the simple GeoJSON taken by the/analyze/
endpoints. This is because of the need to specify layer overrides, and perhaps even more modifiers in the future.The
LAYER_OVERRIDES
definition only documents the__LAND__
and__STREAMS__
layers, since they are the only ones with viable alternatives.The result object is not documented, because:
Connects #3472
Demo
Starting a MapShed job:
Getting job results:
xh --verbose :8000/api/jobs/401cd324-d6dc-4e39-84a0-ec2b2f6491cb/ Authorization:"Token b0c671e1424f37e58c81670ce8118e8eff0f0c14"
Testing Instructions
Token $YOUR_API_KEY
in the box, and click "Authorize"/modeling/gwlf-e/prepare/
endpoint