modified code for byte_size parameter #1846
Conversation
kamalsharma9001
commented
Mar 27, 2024
kamalsharma9001
commented
- Sending byte size limit parameter to bulk processor
- Added a condition to check if total byte size of bulk request has reached byte size limit and accordingly sending bulk request
1. Sending byte size limit parameter to bulk processor 2. Added a condition to check if total byte size of bulk request has reached byte size limit and accordingly sending bulk request
There was a problem hiding this comment.
Thanks a lot for the PR.
I gave a first quick look.
The most important thing to fix is the build. The project does not compile anymore.
I don't think we need to change private final List<T> operations = new ArrayList<>(); by private final List<Object> operations = new ArrayList<>();
I'll do another review when I have some spare time and might propose something.
| @@ -422,9 +423,13 @@ public void index(String index, String id, Doc doc, String pipeline) { | |||
| @Override | |||
| public void indexRawJson(String index, String id, String json, String pipeline) { | |||
| logger.trace("JSon indexed : {}", json); | |||
| //modified code starts | |||
.../src/main/java/fr/pilato/elasticsearch/crawler/fs/framework/bulk/FsCrawlerBulkProcessor.java
Outdated
Show resolved
Hide resolved
| bulkProcessor.add(new ElasticsearchIndexOperation(index, id, pipeline, json)); | ||
| //modified code ends |
There was a problem hiding this comment.
you mean comment written at line 428?
| } | ||
|
|
||
|
|
||
|
|
| @@ -44,9 +44,9 @@ public ElasticsearchBulkResponse bulk(ElasticsearchBulkRequest request) { | |||
| StringBuilder bulkRequest = new StringBuilder(); | |||
| // Header | |||
| bulkRequest.append("{\"") | |||
| .append(r.getOperation().toString().toLowerCase(Locale.ROOT)) | |||
| .append(((ElasticsearchOperation) r).getOperation().toString().toLowerCase(Locale.ROOT)) | |||
.../src/main/java/fr/pilato/elasticsearch/crawler/fs/framework/bulk/FsCrawlerBulkProcessor.java
Outdated
Show resolved
Hide resolved
.../src/main/java/fr/pilato/elasticsearch/crawler/fs/framework/bulk/FsCrawlerBulkProcessor.java
Outdated
Show resolved
Hide resolved
.../src/main/java/fr/pilato/elasticsearch/crawler/fs/framework/bulk/FsCrawlerBulkProcessor.java
Outdated
Show resolved
Hide resolved
.../src/main/java/fr/pilato/elasticsearch/crawler/fs/framework/bulk/FsCrawlerBulkProcessor.java
Outdated
Show resolved
Hide resolved
| @@ -24,17 +24,17 @@ | |||
|
|
|||
| public abstract class FsCrawlerBulkRequest<T extends FsCrawlerOperation<T>> { | |||
|
|
|||
| private final List<T> operations = new ArrayList<>(); | |||
There was a problem hiding this comment.
I don't think we should change that.
There was a problem hiding this comment.
In fr.pilato.elasticsearch.crawler.fs.framework.bulk.FsCrawlerBulkProcessor.java, we were not able to access "getJson" method written in "ElasticsearchIndexOperation" class in "add" method written in FsCrawlerBulkProcessor.java. To achieve this, we changed data type of argument passed to "Object" and later on casted the object for respective class to access it's methods. If you can help in achieving the same in any other way without modifying data type, then it will be helpful
|
You don't need to create another PR for this. You just need to update your branch and push the branch to your repository. It will be automatically managed by GitHub. |
|
|
made changes as per your comments. Please check |
|
This looks overall good. Let me check later this week or next week. |
|
Thanks a lot! I created #1852 from your branch and did then some refactoring + adding some tests. Closing this one then. |
