You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
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.kwargs
(e.g."embedding_dim"
,"method"
,"settings"
) are not mentioned in the docstring. The docstring says that thekwargs
are the ones that theOpenSearch
client takes. AFAIK (please correct me if I'm wrong) the ones extracted are not commonkwargs
required by the client. For example, they are used when creating an index (client.indices.create()
).max_chunk_bytes
argument to pass to the internalbulk
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.
Additional context
Happy to make a PR if you agree on this proposal :)
The text was updated successfully, but these errors were encountered: