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

Add opensearch support #167

Closed jonhealy1 closed 9 months ago

jamesfisher-geo commented 1 year ago

Advice I have seen online suggests to turn on "compatibility mode" in OpenSearch. But I did not have any luck with this approach. I have managed to get OpenSearch working with stac-fastapi-elasticsearch by downgrading elasticsearch[async] from 7.17.9 to 7.13.4, before compatibility checks. This required refactoring the query syntax to an older version. So isn't much use for creating something maintainable on recent releases. For example...

7.17.9

await client.indices.create(
        index=index_by_collection_id(collection_id),
        mappings=ES_ITEMS_MAPPINGS,
        settings=ES_ITEMS_SETTINGS,
        ignore=400,  # ignore 400 already exists code
    )
    await client.close()

7.13.4

await client.indices.create(
        index=index_by_collection_id(collection_id),
        body={"mappings":ES_ITEMS_MAPPINGS,
              "settings": ES_ITEMS_SETTINGS},
        ignore=400,  # ignore 400 already exists code
    )
    await client.close()

As python clients move apart, OpenSearch support would require a separate database_logic.py (and maybe a separate repo?) that is configured to use the opensearch-py library.

jamesfisher-geo commented 11 months ago

Summarizing @philvarner from the STAC Community Meetup:

Something we should probably do is break apart those implementations so that there is one for each and they share almost everything except they have a wrapper around the clients. Like a stac-fastapi-*search repo with interchangeable OpenSearch and Elasticsearch wrapper modules.

A little bit of work to implement but worth it to make completely compatible.

jamesfisher-geo commented 9 months ago

Hey @jonhealy1 I wonder if we can merge our work. I have been working on a fork of your add_opensearch branch here: https://github.com/AtomicMaps/stac-fastapi-elasto-opensearch/tree/add-opensearch

jonhealy1 commented 9 months ago

Hi that would be really good. What are you thinking for how a User chooses which db they want? I was playing around with a couple of things but don't have anything really good yet. The first thing I thought was just an env variable.

jonhealy1 commented 9 months ago

What you have looks really good. I see you're using an env variable to switch too. It's probably the easiest way. I have another branch where I was fooling around with having a separate setup.py for opensearch so we could publish stac-fastapi-opensearch on pypi but I haven't got everything working yet.

jamesfisher-geo commented 9 months ago

Thanks. Yeah, I could not think of a better way than using an environment variable.

Separate setup.py files sounds good. I am finishing up some debugging then should have a version that passes all the tests up in an hour or so

jamesfisher-geo commented 9 months ago

Just pushed updates that pass tests on both ES and OS. How would two setup.py files work?

jonhealy1 commented 9 months ago

What you have looks really good. I am thinking maybe we should merge what you have and then think about going from there. I am not quite sure how it would work right now.

jonhealy1 commented 9 months ago

I have something I am working on now. Three packages. A core package with common code like core.py, then a package each for elasticsearch and opensearch. Basically app.py injects the right DatabaseLogic to the core package. https://github.com/stac-utils/stac-fastapi-elasticsearch/tree/common_core/stac_fastapi

jonhealy1 commented 9 months ago

@jamesfisher-gis I tried to add you to a reviewer on this pr - https://github.com/stac-utils/stac-fastapi-elasticsearch/pull/186. I think you probably need to be added via stac-utils. How do you feel about adding your opensearch changes to this branch?

jamesfisher-geo commented 9 months ago

Nice. I am not in the stac-utils organization yet. I'm happy to join and help with maintaining this project.

I will submit a PR with my changes in an hour and request to merge into the add-opensearch branch.

philvarner commented 9 months ago

Nice. I am not in the stac-utils organization yet. I'm happy to join and help with maintaining this project.

let me see what I can do about that.

jonhealy1 commented 9 months ago

That would be great. Can you add your changes to this branch instead - https://github.com/stac-utils/stac-fastapi-elasticsearch/tree/common_core. I know it's not ideal but I think it's a really good solution.

philvarner commented 9 months ago

Nice. I am not in the stac-utils organization yet. I'm happy to join and help with maintaining this project.

let me see what I can do about that.

Invited you with the Maintain permission level, which should allow you do most anything you need to.

jonhealy1 commented 9 months ago

@philvarner I still can't add James as a reviewer - maybe it's just a temporary glitch

jamesfisher-geo commented 9 months ago

@philvarner I still can't add James as a reviewer - maybe it's just a temporary glitch

I just accepted the invite. It should work now

jonhealy1 commented 9 months ago

Working now!

jonhealy1 commented 9 months ago

I think we should maybe look at merging this before we merge in the opensearch code. https://github.com/stac-utils/stac-fastapi-elasticsearch/pull/177 What do you think? It would mean updating the opensearch logic

jonhealy1 commented 9 months ago

Finishing this opensearch work will be a major accomplishment and we will definitely release a new version of the project.

jamesfisher-geo commented 9 months ago

I think we should maybe look at merging this before we merge in the opensearch code. #177 What do you think? It would mean updating the opensearch logic

Yeah, sounds good. It looks good to me. I will sync those changes with the add_opensearch tomorrow.