Extending temporal search

stac-utils / stac-fastapi-elasticsearch-opensearch

Elasticsearch backend for stac-fastapi with Opensearch support.

https://stac-utils.github.io/stac-fastapi-elasticsearch-opensearch

MIT License

31 stars 15 forks source link

Extending temporal search #182

Open rhysrevans3 opened 10 months ago

rhysrevans3 commented 10 months ago

Related Issue(s):

Description: Extending temporal search include start_datetime and end_datetime properties

PR Checklist:

[x] Code is formatted and linted (run pre-commit run --all-files)
[x] Tests pass (run make test)
[x] Documentation has been updated to reflect changes, if applicable
[x] Changes are added to the changelog

jonhealy1 commented 9 months ago

@rhysrevans3 This pr looks really good. We have just been in the middle of adding opensearch support. Do you have any experience with opensearch?

rhysrevans3 commented 9 months ago

@jonhealy1 I've added some more tests but realised the current version of stac-pydantic the api is using doesn't allow a null datetime field. To update to the latest version of stac-pydantic will require changes to both stac-fastapi and stac-fastapi-elasticsearch. I've made the changes to stac-fastapi in this pull request and am working on the changes to elasticsearch. But I think this pull request will have to wait until they're merged.

rhysrevans3 commented 9 months ago

@rhysrevans3 This pr looks really good. We have just been in the middle of adding opensearch support. Do you have any experience with opensearch?

I don't have any experience with Opensearch but am happy merge main's changes into this branch and try to add this functionality to the opensearch parts.

jonhealy1 commented 7 months ago

@rhysrevans3 Any new thoughts? Hopefully we can merge this soon.

rhysrevans3 commented 7 months ago

@jonhealy1 update on this pull request:

I've added the extended datetime functionality to the opensearch code.
I've added some extra tests to meet all the cases I can think of.
I've added some comments to explain the database logic. Some of the tests I've added are failing as the current version of stac-pydantic doesn't allow the datetime field to be null. So I'm still waiting on the fast-api pydantic v2 pull request to be merged

jonhealy1 commented 6 months ago

Hi @rhysrevans3, the pydantic v2 is merged now into main via stac-pydantic if you want to have another look at this. Cheers.

rhysrevans3 commented 6 months ago

@jonhealy1 it looks like that stac-pydantic still won't allow a null datetime I thought the validator would allow this but I think the typing stops it. https://github.com/stac-utils/stac-pydantic/blob/main/stac_pydantic/item.py#L31-L58

Good news is there appears to be 3 pull requests that will fix this: https://github.com/stac-utils/stac-pydantic/pull/116 https://github.com/stac-utils/stac-pydantic/pull/131 https://github.com/stac-utils/stac-pydantic/pull/135

Sorry for the delay in this.

jonhealy1 commented 6 months ago

Hi @rhysrevans3 - no problems whatsoever - thanks for staying on top of this!

rhysrevans3 commented 5 months ago

@jonhealy1 looks like this might be finally ready to merge. :)

stac-utils / stac-fastapi-elasticsearch-opensearch

Extending temporal search #182

181