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

Adding Collection Search Extension #681

Closed
wants to merge 25 commits into from

Conversation

tjellicoe-tpzuk
Copy link

@tjellicoe-tpzuk tjellicoe-tpzuk commented May 3, 2024

Related Issue(s): #615

Description: Adding Collection Search Extension as defined here

PR Checklist:

  • pre-commit hooks pass locally
  • Tests pass (run make test)
  • Documentation has been updated to reflect changes, if applicable, and docs build successfully (run make docs)
  • Changes are added to the CHANGELOG.

The collection search extension adds two endpoints which allow searching of
collections via GET and POST:
GET /collection-search
POST /collection-search
Copy link
Member

Choose a reason for hiding this comment

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

we should add a note that the extension seems to mention GET /collections and POST /collections but due to possible conflict with the transaction extension we should to use /collection-search

also wonder if we should use /collections-search or /search/collections 🤷‍♂️

@@ -175,7 +177,7 @@ def register_get_item(self):
"""
self.router.add_api_route(
name="Get Item",
path="/collections/{collection_id}/items/{item_id}",
path="/catalogs/{catalog_id}collections/{collection_id}/items/{item_id}",
Copy link
Member

Choose a reason for hiding this comment

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

😬 maybe this change wasn't made on purpose?

Copy link
Contributor

@m-mohr m-mohr left a comment

Choose a reason for hiding this comment

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

This doesn't implement https://github.com/stac-api-extensions/collection-search

The extension doesn't define a POST /collections endpoint, only GET /collections. The endpoint names are wrong.

All functionality in OGC API - Records - Part 1: Local Resource Catalogue is only defined for the GET method (i.e. GET /collections). Note: STAC may add behavior for POST /collections in the future, but due to a potential conflict with the Transaction Extension, specific rules for content negotiation might be required.

@tjellicoe-tpzuk
Copy link
Author

Thanks for your comments @vincentsarago and @m-mohr, this PR has become mixed up with subsequent commits, I will raise a new one only containing the updates for the collection search extension - see draft PR #693

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.

3 participants