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

Improve init arguments of OpenSearchDocumentStore #671

Closed
EdAbati opened this issue Apr 18, 2024 · 0 comments · Fixed by #739
Closed

Improve init arguments of OpenSearchDocumentStore #671

EdAbati opened this issue Apr 18, 2024 · 0 comments · Fixed by #739
Assignees
Labels
feature request Ideas to improve an integration integration:opensearch P2

Comments

@EdAbati
Copy link
Contributor

EdAbati commented Apr 18, 2024

Is your feature request related to a problem? Please describe.

I think that the current implementation of OpenSearchDocumentStore should be more specific about which arguments it accepts in the __init__ method.

  • Some specific keys extracted from the kwargs(e.g. "embedding_dim", "method", "settings") are not mentioned in the docstring. The docstring says that the kwargs are the ones that the OpenSearch client takes. AFAIK (please correct me if I'm wrong) the ones extracted are not common kwargs required by the client. For example, they are used when creating an index (client.indices.create()).
  • It'd be nice to have also a max_chunk_bytes argument to pass to the internal bulk function. The default 100MB may be too big for certain Openserach instances. In these cases, the current implementation raises an error.

Describe the solution you'd like

I suggest adding these arguments explicitly to the __init__ and to improve its docstring.

e.g.

def __init__(
        self,
        *,
        hosts: Optional[Hosts] = None,
        index: str = "default",
        embedding_dim: int = ...,
        method: ...,
        max_chunk_bytes: ...,
        **kwargs,
    ):

Additional context
Happy to make a PR if you agree on this proposal :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Ideas to improve an integration integration:opensearch P2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants