Add opensearch support
Closed this issue · 20 comments
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.
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.
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
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.
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.
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
Just pushed updates that pass tests on both ES and OS. How would two setup.py files work?
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.
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
@jamesfisher-gis I tried to add you to a reviewer on this pr - #186. I think you probably need to be added via stac-utils. How do you feel about adding your opensearch changes to this branch?
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.
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.
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.
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.
@philvarner I still can't add James as a reviewer - maybe it's just a temporary glitch
@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
Working now!
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
Finishing this opensearch work will be a major accomplishment and we will definitely release a new version of the project.
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.