v.1.0.0-rc.1 release merge to master #291
Conversation
#235) * Filter Extension - add Accent and Case-insensitive Comparison conformance class
* remove references to query extension being deprecated
…te npm build (#261) * update CQL2 definition to reference OGCAPI yaml file in github * put related issue in list to get rich formatting
…inks require children endpoint to return all child link objects
* update CQL2 implementation suggestions
…oll-for-children explicitly state /children may return fewer fields for its entities
clarify wording of search link rel requirement
…nce class names with italics
change all uses of shall to must, and stylize all non-linked conformance classes
…ce-class-consistency Pv/filter extension conformance class consistency
update 1.0.0 beta.5 to rc.1
…l-and-remark-upgrade fix a few minor openapi spec issues, upgrade remark and spectral
* update Transaction Extension to more closely match OAFeat Part 4, disallow changing collection or id in PUT or PATCH * txn ext: add id and collection population language * update body for txn post, put, and patch
philvarner
requested review from
cholmes,
matthewhanson,
lossyrob,
m-mohr and
jbants
|
I think I don't agree with this release plan. Shouldn't all major issues being resolved for an rc.1? Especially the repo split I'd consider like it should be part of rc.1 as quite a number of things could go wrong and potentially also the conformance classes (i.e. how the URLs looks like) could change drastically. Having such changes on the roadmap is an indicator for a beta for me, not for a rc. Would like to discuss in a broader group (e.g. the PSC or STAC meeting), I think. We also did the switch for rc.1 in stac-spec, I think. I'm also wondering whether we should do an issue triage where we go over them all once and check whether they should be included in 1.0 or not, e.g. thinking about #239 and #250. |
| - PUT and PATCH of a body that changes the `collection` or `id` is disallowed. | ||
| - POST, PUT, and PATCH do not need to include the `collection` attribute, as it should be derived from the URL. | ||
| - POST and PUT can be used with a body that is at least a GeoJSON Feature, but does not have to be an Item, but for which | ||
| the server can derive a valid Item, e.g., by populating the id and collection fields or adding links |
There was a problem hiding this comment.
does this mean the geojson feature has to have an id field?
There was a problem hiding this comment.
good question -- I had thought geojson feature required id, but now I see it's a "should". So maybe we need to clarify that behavior -- maybe just say that it's up to the implementer if they want to autogenerate IDs.
There was a problem hiding this comment.
Decided the answer to this is "yes". The openapi spec already defined id as a required field.
|
|
||
| When the body is a partial Item: | ||
|
|
||
| - Shall only create a new resource. |
There was a problem hiding this comment.
Should 'shall' be taken out here as well?
There was a problem hiding this comment.
yeah, I copied this from OAFeat, so this should be must.
|
|
||
| ### POST | ||
|
|
||
| When the body is a partial Item: |
There was a problem hiding this comment.
A geojson feature would be a partial item I think?
There was a problem hiding this comment.
yes, mostly. geojson features don't have to have an id field, so this item either has to have an id field or the server has to support autogenerating one.
| - Shall return 201 and a Location header with the URI of the newly added resource for a successful operation. | ||
| - May return the content of the newly added resource for a successful operation. | ||
|
|
||
| When the body is a partial ItemCollection: |
There was a problem hiding this comment.
an ItemCollection that contains partial Items? What is a partial ItemCollection?
There was a problem hiding this comment.
It's defined in the OpenAPI spec -- it's the same as an ItemCollection, but features contains partial Items (e.g., don't have to have links, etc). The gist of the partial Item/ItemCollection is that you don't have to submit a valid Item, it just has to be enough of an Item that the server can synthesize the missing elements -- for example, collection can be populated from the endpoint it was posted to, only of one of bbox or geometry needs to be set, etc.
| - Shall return 409 if an Item exists for any of the same collection and id values. | ||
| - Shall populate the `collection` field in each Item from the URI. | ||
| - Shall return 201 without a Location header. | ||
| - May create only some of the Items in the ItemCollection. |
There was a problem hiding this comment.
Maybe more explanation. When would only some Items be created?
There was a problem hiding this comment.
For example, there are two items and the second one already exists with that ID -- the first gets created, then the second fails. We can't guarantee transactional semantics, so now the first one exists in the catalog but the second one doesn't.
Probably a good idea to clarify this more in rc.2. Also, it would be good to have a better description of the recomended behavior -- perhaps return 200 with an ItemCollection of the ones that were successful, and an error field with the ids (or something else if the problem was with the id).
|
@m-mohr that's fine, but I'm not able to do any more of this work. I've been doing this in my spare time for the last two months, and I was just trying to get an rc.1 out. If you or the PSC think any of this work should be put in for rc.1, you'll need recruit people to do it. |
|
@m-mohr also, I don't think the conformance class URIs will change just because we put them in different repos. We should keep them the same, and maybe change the convention for new ones rather the make the breaking change with these. |
|
@philvarner I can totally relate to that as I've also contributed a fair amount of spare time to stac-spec work and all your work is appreciated! There's a PSC meeting on Monday so I think we can discuss this promptly. About the conformance classes: I'm not 100% sure about that, all the URLs had changed when we moved the stac extensions. I guess the basic question is whether these URIs are "virtual" or whether there's something behind it that needs to be deployed by CI. Then it gets tricky because the CI of this repo would need to deploy stuff from external repos, right? But maybe I'm on a completely wrong track here. |
The OGC precedent is very much that they are 'virtual' - they are just a bundle of functionality. Indeed the OGC stuff is annoying because they are technically URI's (I believe), not resolving to anything (like a URL does). So if you just stick an OGC conformance class URI in it often doesn't exist as a webpage at all. I'd like to do something better, but right now we're not doing anything either. The core stac-spec stuff wasn't trying to follow OGC conformance class style stuff. Those were actual JSON schemas, and we wanted to deploy them from the repos. Looking at OGC API features it looks like their conformance classes URI's do resolve, but they are just a redirection to the published specification. So I think it's fine if we move things out and then figure out how to get all the redirections right later. They deploy to api.stacspec.org, so it's not actually tied to the repository. I agree there may be some advanced CI work to make it all happen automatically. But I believe it's fine to just not have some of the conformance classes resolve - they are just a communication of the bundle of functionality, not a contract that the URL will be the openapi docs (as it is right now). Indeed I could see us choosing to redirect to the actual spec (like features API does, though they publish the spec instead of pointing direct at the github repo), or even nice documentation that explains each conformance class in more introductory texxt. |
|
Okay, I also want to ensure clients don't need to check too many conformance URIs. Right now in STAC Browser I'm doing regexp matching (like |
|
As long as the conformance class URIs still have the same structure after the restructure of the extensions, then I'm good to release rc.1 before the restructuring. Haven't reviewed yet though, so no approval yet (but also no change requests so far). |
|
Let's move the topics @jonhealy1 raised to their own issues and resolve all conversations, and then this should be good to merge. Discussed at PSC meeting and everyone is good with figuring out extensions locations after rc.1, since the conformance URI's are virtual and will stay the same. |
|
Captured actionable comments in #292, @jonhealy1 feel free to update if I missed anything. |
|
🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 |
No description provided.