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

Rework API examples page #483

Merged
merged 2 commits into from
Jun 10, 2019
Merged

Rework API examples page #483

merged 2 commits into from
Jun 10, 2019

Conversation

joshfix
Copy link
Contributor

@joshfix joshfix commented Jun 7, 2019

The API examples page primarily focused on dynamic queries against the /collections/{collection_id}/items endpoint, which is limited to only the limit, time, and bbox parameters. Because of the limitations, we should be driving STAC users who wish to perform dynamic queries to the /stac/search endpoint.

In this PR, the examples have been reworked to show examples against the /stac/search endpoint. A single example has been left to show how to query the collections endpoint and outline that the collections endpoints does not support HTTP POST.

@joshfix joshfix requested review from matthewhanson and m-mohr June 7, 2019 14:02

## Core examples

Requests all the data in the collection that is in New Zealand:
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe call this explicitly Items rather than data -- since technically all STAC entities are metadata

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good

POST /stac/search
```

Body:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd prefer if we required this to be multipart/form-data instead of a JSON query body.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I prefer that because all of the values passed to the parameters are exactly the same in the GET and POST cases. Otherwise, you have to define a separate query payload JSON schema, with similar (but not exact) mappings between values, for example, that ids is an array of strings rather than a comma-separated list of strings. With a JSON payload, we also get into the tricky situation when using the query extension where the value of one of the attributes in the JSON is a string representing another query language, e.g., CQL or GraphQL, which can expose all sorts of bugs in JSON implementations (similarly to how our use of : in extension prefixes do)

I see more APIs that use this convention. for example, https://developer.twitter.com/en/docs/tweets/search/api-reference/enterprise-search#Methods

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would need to be filed as a separate issue. From the beginning we have used raw application/json. This examples page is examples of the current spec.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah right, will do that.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Created a PR for some explicit language around supporting POST. #489

@matthewhanson
Copy link
Collaborator

This looks good to me. Like the idea of @philvarner about multipart, but agree that should be different PR.

I won't merge quite yet, in case @joshfix wants to tweak the verbiage somewhat based on the suggestions by @philvarner

Signed-off-by: Josh Fix <josh@federal.planet.com>
@joshfix
Copy link
Contributor Author

joshfix commented Jun 10, 2019

I updated the verbiage to say "items" instead of "data". Should be good to go.

@cholmes cholmes merged commit 40b9d1b into radiantearth:master Jun 10, 2019
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.

5 participants