Problem/Motivation
We use this module to do processing of CSV's into Elasticsearch but our ES instance is behind a nginx proxy which does some logic based upon the index name in the URI. In past versions, since indexing was done document by document, this was fine. The latest version now correctly moved to the bulk endpoint, but by default, the bulk endpoint puts a list of indices in the body rather than the URI - the motivation being that you can bulk to multiple indices.
This implementation however will only currently push data into a single index via it's pipeline config.
The bulk endpoint does allow specification of a single index in the URI, which is a more correct way of using bulk if you only intend to use one index for your bulk import rather than many.
Ref: https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-bul...
Steps to reproduce
There isn't a reliable method to reproduce this specific use case, more a correction of the implementation for it to be true to it's single index requirements.
Proposed resolution
I propose moving the specified index out of the body and into the URI, by moving it out of the body array in the DatasetDestination implementing class and adding it to the params array.
Remaining tasks
Check to see if tests are required for this extension?
User interface changes
N/A
API changes
N/A
Data model changes
N/A
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | change-bulk-indexing-3385068-5.patch | 1.82 KB | nadim hossain |
Issue fork data_pipelines-3385068
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3385068-change-bulk-indexing
changes, plain diff MR !11
Comments
Comment #3
larowlanSeems reasonable to me - RTBC assuming tests come back green
Comment #4
jibranLGTM as well. Thanks for the MR.
Comment #5
nadim hossain commentedAdding this patch file to use it in the composer patch instead of using the merge request diff until the merge request is merged and released.
Comment #7
larowlanSorry about the delay here, this one slipped off my radar.
Cutting a release now
This will go out in alpha22