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

[PROPOSAL] Plugins Python Client Interface #90

Closed
arnavdas88 opened this issue Nov 13, 2021 · 9 comments
Closed

[PROPOSAL] Plugins Python Client Interface #90

arnavdas88 opened this issue Nov 13, 2021 · 9 comments

Comments

@arnavdas88
Copy link
Contributor

arnavdas88 commented Nov 13, 2021

What kind of business use case are you trying to solve? What are your requirements?

Although the current state of opensearch-py can connect to opensearch database and do transactions seamlessly, To use plugins features like Search Plugin or Monitoring Plugin, one needs to follow the REST guideline from docs and need to make the requests and the workflow manually. I, right now have a personal requirement of the alerting feature from the Monitoring Plugin to be implemented in a pythonic way.

What is the problem? What is preventing you from meeting the requirements?
The problem is the lack of python support for the plugins as most of the organizations using the python client would probably use monitoring and other plugin features as well. Lack of this particular feature would mean, one needs to use opensearch-py in one hand and also using the REST API from the docs on the other hand, creating a mixed feeling in the developers heart, unhealthy code and probably some security flaws in the developer's code as well.

What are you proposing? What do you suggest we do to solve the problem or improve the existing situation?
I am proposing a pythonic implementation of the plugins API, which would look something like this:

from opensearchpy import OpenSearch
from opensearchpy.plugins.alerting import Monitors, MonitorType, Trigger

client = OpenSearch(...)

# To Get Monitor from it's ID
client.plugins.alerting.get_monitor('X9umLn0B5FPOmJ6VSaWe')

# To Create a new Monitor
query = {...}
client.plugins.alerting.create_monitor(query)

# To get all Destinations
client.plugins.alerting.get_destination()

...

What are your assumptions or prerequisites?
N/A

What are remaining open questions?
List questions that may need to be answered before proceeding with an implementation.

  • Is this feature contextual to this particular project or is it more contextual to opensearch-dsl-py or should someone make a different project to interface the plugins?
  • Assuming that this proposal is contextual to either this project or the dsl one, what kind of directory structure should I proceed with. Should I add a directory named contrib or plugins ?
  • Should I go for as much pythonic way I can or should I ask most of the input arguments as Dictionary?
  • Any coding convention that I should be following ?
@arnavdas88 arnavdas88 mentioned this issue Nov 20, 2021
1 task
@stockholmux
Copy link
Member

@axeoman and @rushiagr What are your thoughts on this one? Seems like a good plan to me.

@axeoman
Copy link
Collaborator

axeoman commented Dec 3, 2021

Yea, I agree that this is a useful feature and thanks a lot for that 👍

@VijayanB
Copy link
Member

Do we really need to have "plugins" between client and alerting. Why not have following format?

client.alerting.get_destination()

@dblock can you provide some feedback?

@arnavdas88
Copy link
Contributor Author

@VijayanB

1.) I was thinking of following this structure in the image below, which means client.plugins.alerting and later client.plugins.reporting, client.plugins.notebooks, etc...

alerting

2.) Instead, we can always go for the structure client.alerting and later client.sql, client.knn, etc...

alerting


Which one do you think we should go for?

@dblock
Copy link
Member

dblock commented Dec 31, 2021

The first question is whether there's a single opensearch-py or opensearch-py and opensearch-alerting-py. If we believe in a vision that OpenSearch is highly extensible, plugins will exist independently, and there will be a version matrix for compatibility where a plugin says "I am compatible with these versions of OpenSearch". Similarly, this means that opensearch-py should not know about plugins, but vice-versa. I would want to add opensearch-alerting-py to my project, which may depend (be compatible with) a range of versions of opensearch-py. I do think it's ok for all these plugin implementations to live in this project until we start shipping clients separately, too, but we just need to be aware of that future.

In terms of namespaces, I would definitely keep the namespace plugins for the code, so from opensearchpy.plugins.alerting import Monitors LGTM, but it does feel a bit weird to have to write client.plugins.alerting when using it. However, if we don't use plugins, I cannot have an index plugin today because client.index is taken. I would implement both client.plugins.alerting and client.alerting via dynamic lookup, so that if there is a conflict I can disambiguate. Another variation of this is when a plugin interface is loaded, check whether the method exists, warn, and skip making client.index=client.plugins.index.

Some more thoughts ...

I love the idea of extending the client to plugins very much, and definitely appreciate when clients are implemented the language-way (e.g. mongoid was a lot easier to use than a bare-bones mongodb-ruby-driver and eventually became the official client)), as opposed to something that one generates that gets you 80% there. I also really appreciate when clients do a little bit more than the pure API to make the caller's life easier (e.g. when you ask for lists in slack-ruby-client with a block, it automatically paginates results for you keeping track of pages and making additional API requests for you). With this in mind, it seems like https://github.com/opensearch-project/opensearch-dsl-py tries to be such a client for many types of search queries. Making it easy to make calls to plugins in the same vein is a great suggestion.

I think it's worth stepping back a little and imagining what the ideal world could look like.

  1. OpenSearch and all its plugins publish their API schema using a modern toolset and keep it up-to-date. This is OpenSearch Smithy IDL Model - RFC OpenSearch#1590. In my above-mentioned example for slack-ruby-client the output of such a process is https://github.com/slack-ruby/slack-api-ref.
  2. Language clients, including this Python client, consume (1) to generate all the boilerplate code. Customers who don't want anything but the API would be able to do it themselves. In the python client this means things like https://github.com/opensearch-project/opensearch-py/blob/main/opensearchpy/client/cat.py would be generated, not authored by hand. Updating the API and shipping a new client is super easy.
  3. Language-specific implementation extends the generated code to make it super easy to use, providing the current interface.

I support anything that gets us closer to this.

@gabrielcocenza
Copy link

Hello 👋

I'm currently developing a Juju charm for OpenSearch and I'm missing a way of setting CCR (Cross Cluster Replication) via the python client. Right now I'm using the rest API in the documentation to do it, but it raises some concerns that the code might brake in the future if the endpoints changes between different OpenSearch versions.

Reading the code of the ElasticSearch client they do have a ccr client that I think would be good to have it also on Opensearch.

@stockholmux
Copy link
Member

@gabrielcocenza The API is the way to do it right now. OpenSearch follows SemVer so the concern of the APIs breaking will only be an issue at a major versions. I would think the charm you're developing would need to change at that time anyway.

Also, the ES client's CCR is similar in name only - I don't believe that theres is compatibility between the OpenSearch and ES implementations of CCR.

All this being said, I agree though, yes, going forward it would be nice to have CCR abstracted into the library :)

@divyankm
Copy link

This would be great if this happens. Putting curl requests using python is bit cumbersome and also facing issues.

https://forum.opensearch.org/t/unable-to-create-monitors-using-curl-request-error-unauthorized/10097

@dblock
Copy link
Member

dblock commented Nov 10, 2023

A mechanism for plugins was added in #93. Closing.

@dblock dblock closed this as completed Nov 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants