fapaul commented on a change in pull request #18038:
URL: https://github.com/apache/flink/pull/18038#discussion_r763810541



##########
File path: 
flink-test-utils-parent/flink-test-utils-junit/src/main/java/org/apache/flink/util/DockerImageVersions.java
##########
@@ -24,11 +24,9 @@
  * as well as to provide a central file to use as a key when caching testing 
Docker files.
  */
 public class DockerImageVersions {
-    public static final String ELASTICSEARCH_7 =

Review comment:
       If we remove this version we can probably rename the other variable.

##########
File path: flink-connectors/flink-sql-connector-elasticsearch6/pom.xml
##########
@@ -135,6 +135,10 @@ under the License.
                                                                        
<pattern>com.fasterxml.jackson</pattern>
                                                                        
<shadedPattern>org.apache.flink.elasticsearch6.shaded.com.fasterxml.jackson</shadedPattern>
                                                                </relocation>
+                                                               <relocation>
+                                                                       
<pattern>com.github.mustachejava</pattern>
+                                                                       
<shadedPattern>org.apache.flink.elasticsearch6.shaded.com.github.mustachejava</shadedPattern>
+                                                               </relocation>

Review comment:
       Sorry for asking again but why the relocation? 

##########
File path: 
flink-test-utils-parent/flink-test-utils-junit/src/main/java/org/apache/flink/util/DockerImageVersions.java
##########
@@ -28,11 +28,8 @@
     public static final String ELASTICSEARCH_COMMERCIAL_7 =
             "docker.elastic.co/elasticsearch/elasticsearch:7.15.2";
 
-    public static final String ELASTICSEARCH_6 =
-            "docker.elastic.co/elasticsearch/elasticsearch-oss:6.3.1";
-

Review comment:
       Same as for 7 we can subsume the OSS version with the commercial one.

##########
File path: flink-connectors/flink-sql-connector-elasticsearch7/pom.xml
##########
@@ -142,6 +144,10 @@ under the License.
                                                                        
<pattern>com.github.mustachejava</pattern>
                                                                        
<shadedPattern>org.apache.flink.elasticsearch7.shaded.com.github.mustachejava</shadedPattern>
                                                                </relocation>
+                                                               <relocation>
+                                                                       
<pattern>net.jpountz</pattern>
+                                                                       
<shadedPattern>org.apache.flink.elasticsearch7.shaded.net.jpountz</shadedPattern>
+                                                               </relocation>

Review comment:
       Can you explain this relocation?

##########
File path: 
flink-connectors/flink-connector-elasticsearch-base/src/main/java/org/apache/flink/streaming/connectors/elasticsearch/ElasticsearchApiCallBridge.java
##########
@@ -73,6 +73,16 @@
     @Nullable
     Throwable extractFailureCauseFromBulkItemResponse(BulkItemResponse 
bulkItemResponse);
 
+    /**
+     * Sets the bulk flush interval, in milliseconds on the provided {@link 
BulkProcessor.Builder}.
+     * The builder will be later on used to instantiate the actual {@link 
BulkProcessor}.
+     *
+     * @param builder the {@link BulkProcessor.Builder} to configure.
+     * @param flushIntervalMillis the flush interval in milliseconds.
+     */
+    void configureBulkProcessorFlushInterval(

Review comment:
       Why do we add this configuration option to the old connector?

##########
File path: docs/content/docs/connectors/datastream/elasticsearch.md
##########
@@ -43,11 +43,11 @@ of the Elasticsearch installation:
   </thead>
   <tbody>
     <tr>
-        <td><= 6.3.1</td>
+        <td>6.x</td>
         <td>{{< artifact flink-connector-elasticsearch6 >}}</td>
     </tr>
     <tr>
-        <td><= 7.5.1</td>
+        <td>7.x</td>

Review comment:
       Why do we switch from real versions to only describing the major version?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to