-
Notifications
You must be signed in to change notification settings - Fork 639
Open
Description
Hi all,
I've been messing around with this library and came across that NewBulkIngestor does not seem to return an error in any case. The only place where an error could be obtained, when creating the Elastic client if it is nil, the error message is completely ignored.
I had a few thoughts;
- If that function is never going to return an error, the func signature should change so the user isn't expected to handle an error that will never occur
- I personally don't understand the reason why creating a default Elastic client is desirable if it is nil, I would expect that to be an error along the lines of "elastic client is nil" so that the user is told to handle the making of a new client. A bulk indexer init func, in my mind, should not make that decision.
- If the ability for the client to be made if it is nil is still desirable, then we should at least let that error from the
elasticsearch.NewDefaultClient()call be returned if the function is going to declare it could return an error.
Happy to make whatever change is decided. In my opinion, we should forego the making of that default client (unless there is a use case I'm missing) and return an error in the case that cfg.Client == nil.
Thanks all for your time.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels