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

Feature/add collection search extension V2 #736

Merged
merged 6 commits into from
Jul 22, 2024

Conversation

vincentsarago
Copy link
Member

@vincentsarago vincentsarago commented Jul 19, 2024

simpler version of #681

This PR doesn't add the/collections - POST endpoint because:

  • it's not mandatory in the spec
  • it will conflict with the transaction extension 😬

We could in another PR show (in the documentation) how to create a /collections - POST which support both search and transaction

cc @tjellicoe-tpzuk

@vincentsarago
Copy link
Member Author

this is ready for review

Copy link
Collaborator

@jonhealy1 jonhealy1 left a comment

Choose a reason for hiding this comment

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

Cool beans!

CHANGES.md Outdated Show resolved Hide resolved
CHANGES.md Outdated Show resolved Hide resolved
@jonhealy1 jonhealy1 self-requested a review July 22, 2024 22:34
@m-mohr
Copy link
Contributor

m-mohr commented Aug 1, 2024

@jonhealy1 @vincentsarago I had another look at this and I'm wondering where the queryables enpoint is exposed. The filter extension, which is listed in the conformance classes, would require to be able to query based on queryables but I don't see where the link is added. Is that done in the backends?

@vincentsarago
Copy link
Member Author

Yes that should be done by the users in their application.

@m-mohr
Copy link
Contributor

m-mohr commented Aug 1, 2024

@vincentsarago Why though? If you have the filter extension enabled, you need the queryables link anyway. Could return an empty schema, but why this manual step?

Related PR: stac-utils/stac-fastapi-pgstac#89

@vincentsarago
Copy link
Member Author

@m-mohr I'm not quite sure what you are asking to be honest. From the extension POV we cannot enforce the links to be present in the endpoints response 🤷

The /queryables endpoints in added to the application when you register a FilterExtension to the application.

if you add the FilterExtension GET model to the collection-search model (as in https://github.com/stac-utils/stac-fastapi/pull/736/files#diff-6b36639e106da8e405a2aeb7d41d08853d6688ff9b34c740309da5d22ac2816bR89-R102) it's then up to the user to also register the FilterExtension to the application itself to have the /queryables endpoint

@m-mohr
Copy link
Contributor

m-mohr commented Aug 1, 2024

@vincentsarago I'm missing the queryables link (not the endpoint itself) in GET /collections, similar to how it's missing for global Item Search in GET / (see stac-utils/stac-fastapi-pgstac#89 ). Maybe I'm missing something though?

If it needs to be added in the back-ends (as in the mentioned PR, although stac-utils/stac-fastapi-pgstac#89 (comment) is confusing then) it's fine, just feels conceptually a bit weird.

@vincentsarago
Copy link
Member Author

👍 the links are added in the client as you did in the stac-FastAPI-pgstac PR

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