-
Notifications
You must be signed in to change notification settings - Fork 47
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
Minor clean-up and more intro #58
Conversation
api-spec.md
Outdated
@@ -40,6 +54,23 @@ Implementations may **optionally** provide support for the full superset of STAC | |||
where the collection ID in the path is equivalent to providing that single value in the `collections` query parameter in | |||
STAC API. | |||
|
|||
### STAC API Compliance |
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.
How to expose only STAC Collections in an API? This would make 10+ implementations in openEO not STAC API compliant any more!
In general, I changed my mind on this since commenting in #42. I think we should only require the landing page and from there just look what has been provided in the links. The issue is that clients will now expect that all those endpoints are accessible, but for (partly) private APIs this is not the case. For example, Radiant Earth has only /, /collections and /collections/:id publicly available and then the others need authentication. So by just pointing a client without credentials to this API, it will fail on all requests that require authentication, which is the same as just not having them implemented (like in openEO). If clients follow the RESTful approach, they just follow links and don't necessarily need any pre-defined endpoint to exist (except for a landing page).
Also, I think there's a mismatch in this PR: Either its RESTful as specified in the introduction (and thus only links are required to navigate the API, no specific endpoints required other than a landing page) OR we require certain endpoints, but then it's not strictly RESTful any longer IMHO.
If we stick with this requirement, it must be mentioned in the Changelog as this is a major change compared to 0.9.
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.
I'm open on what exactly we do here, was just trying to capture what was in the PR.
My thinking was that a Collection only endpoint is actually not a STAC API. In my mind a STAC API is when you provide 'search'. OpenEO is a compliant STAC catalog, as is https://storage.googleapis.com/pdd-stac/disasters-0.9.0/catalog.json And I'd see implementations that are servers (RESTful API's) that only implement STAC core, and don't offer search/ or collections/items/ endpoints.
I think I agree with your point about RESTful, with no specific endpoints required, but we should be able to still specify functionality required in some way? Is OGC Features API RESTful?
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.
OpenEO is a compliant STAC catalog, as is https://storage.googleapis.com/pdd-stac/disasters-0.9.0/catalog.json
Unfortunately, it is not. We use the data rel type instead of child as it is specified in STAC API. So it's actually not a valid STAC catalog... And then /collections is also not valid STA, it's only valid in STAC API.
I think I agree with your point about RESTful, with no specific endpoints required, but we should be able to still specify functionality required in some way?
Why would that be required? If we have conformance classes anyway, couldn't we just specify what is implemented there or just follow links? I'm not sure myself yet, but I don't like that it's obviously getting towards just STAC API for Items and it seems I need to come up with my own STAC API spec for Collections. I mean it's a bit an issue of course that OGC API Features requires the item endpoints, too, and as such I'm basically only compliant to OGC API Commons, but then have STAC Collections of course. Anyway, we can't represent all static STACs in the API it seems...
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.
Ah, that's interesting on using data rel type instead of child, I'd thought they were child. I do think that's a confusing little area.
I'm not set on not having STAC API just for items, just saying that was more of my mental model. That everyone should implement STAC Core, and that you layer on Features API / Search endpoint for filtering items. And the latter is 'STAC API'.
But for beta.1 I prefer to not rock the boat - mostly just want to align with CQL / Transactions, clean things up, and get something for people to work with. I definitely see a beta.2, and I think the recommendations for how to implement STAC catalogs / collections is core to figure out, and this to me fits in. I think the conformance classes are probably the way to go. And I'm not set on my mental model, I can see value in having 'STAC API' specify how to implement Collections / Catalogs without search. But I also like that STAC Core can be implemented by a dynamic server, but that we don't need to specify that.
Anyway, we can't represent all static STACs in the API it seems...
What does this mean?
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.
Ah, that's interesting on using data rel type instead of child, I'd thought they were child. I do think that's a confusing little area.
It makes sense, because it returns you a different JSON. Not a collection/catalog, but the response of /collections, which contains collections and links. /collections then lists the "children" instead of adding (in our case) 450+ links to the links in the landing page. It's just that this is not specified in STAC itself, but only in API (originating from Features).
I think the conformance classes are probably the way to go
Agreed, see #27.
I can see value in having 'STAC API' specify how to implement Collections / Catalogs without search.
Actually, I'd rather say we should have collection search at some point and then there might be no "without search" anylonger.
Anyway, we can't represent all static STACs in the API it seems...
What does this mean?
If we merge this PR, you need Items in collections/catalogs, otherwise the API is not compliant although it's a valid STAC.
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.
Hmm, my statement about adding child links to root is a bit biased by openEO and is also weakened by the discussion on Monday. I don't think there's a major issue with your implementation. child links at the landing page are fine, BUT I'd recommend to put them in /collections and link to it with rel type data if OAFeat is implemented anyway. Also, in an API that is not necessarily only STAC (as openEO is), it would bloat the initial request with data that is not required at that stage and thus connection times can slow down. In an API just covering STAC, I'd argue it's not a major problem as it's the primary goal/content of the API.
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.
I think I'd just remove the whole new chapter for now, merge this and then write-up the conformance classes. Those will basically change the whole chapter anyway.
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.
My main interest is in having the API browsable by STAC browser, so I think that will need an update to respect the data
rel type link.
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.
Yes, that was my plan - remove the whole chapter on compliance, and then redo it with the conformance PR.
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.
@matthewhanson I implemented the data rel type some days ago into STAC Browser: radiantearth/stac-browser#59 Already live on stacindex.org...
api-spec.md
Outdated
|
||
A typical OAFeat will have multiple collections, and each will just offer simple search for its particular collection at | ||
`GET /collections/{collectionId}/items`. | ||
The main STAC endpoint specified beyond what OAFeat offers is `/search`, which offers cross-collection search. A primary | ||
use case of STAC is to search diverse imagery collections (like Landsat, Sentinel, MODIS) by location and cloud cover for any | ||
relevant image. So the ability to do searches across Collections is required, and is not yet specified by OAFeat. Due to the | ||
relevant image. So the ability to do searches across Collections is very important, and is not yet specified by OAFeat. Due to the |
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.
"for any relevant image" -> "for any matching Items."
"not yet specified by OAFeat" suggests this is an upcoming capability, we should avoid making assumptions about the future.
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.
They have identified it as future work (in opengeospatial/ogcapi-features#451), and reference it in Filter/CQL - http://docs.opengeospatial.org/DRAFTS/19-079.html#filter-param-multiple-collections
api-spec.md
Outdated
@@ -40,6 +54,23 @@ Implementations may **optionally** provide support for the full superset of STAC | |||
where the collection ID in the path is equivalent to providing that single value in the `collections` query parameter in | |||
STAC API. | |||
|
|||
### STAC API Compliance |
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.
Catching up, think this all was largely resolved in Monday's discussion.
However, regarding @m-mohr statement about not adding child links to root catalog when you have too many collections....do we have another open discussion on this? In stac-cmr there are a few providers that have more than 1000 collections, and currently they are all added to the root endpoint, e.g., https://cmr.uat.earthdata.nasa.gov/stac/GES_DISC
Failing CircleCI, problem with markdown somewhere, I think I see where the problem is, pushing commit |
CHANGELOG.md
Outdated
@@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 | |||
## \[Unreleased\] | |||
|
|||
### Added | |||
- More intro text |
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.
I don't think this needs to be in the changelog and might even confuse people.
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.
Fair. I was going back and forth on it. Wouldn't have added it except I already had a line there. Will take it out.
Related Issue(s): #42
Proposed Changes:
PR Checklist:
npm run generate-all
to update the generated OpenAPI files.