Jackie-Jiang commented on a change in pull request #8390:
URL: https://github.com/apache/pinot/pull/8390#discussion_r836642020



##########
File path: 
pinot-common/src/main/java/org/apache/pinot/common/utils/http/HttpClient.java
##########
@@ -294,14 +329,23 @@ public SimpleHttpResponse sendMultipartPostRequest(String 
url, String body)
       return new SimpleHttpResponse(statusCode, 
EntityUtils.toString(response.getEntity()));
     }
   }
-
   public SimpleHttpResponse sendMultipartPutRequest(String url, String body)
       throws IOException {
+    return sendMultipartPutRequest(url, body, null);
+  }
+
+  public SimpleHttpResponse sendMultipartPutRequest(String url, String body, 
@Nullable Map<String, String> headers)
+      throws IOException {
     HttpPut put = new HttpPut(url);
     // our handlers ignore key...so we can put anything here
     MultipartEntityBuilder builder = MultipartEntityBuilder.create();
     builder.addTextBody("body", body);
     put.setEntity(builder.build());
+    if (headers != null) {

Review comment:
       (minor)
   ```suggestion
       if (MapUtils.isNotEmpty(headers)) {
   ```

##########
File path: pinot-connectors/pinot-flink-connector/pom.xml
##########
@@ -71,6 +57,12 @@
       <version>${flink.version}</version>
     </dependency>
 
+    <dependency>
+      <groupId>org.apache.pinot</groupId>
+      <artifactId>pinot-controller</artifactId>
+      <version>${project.parent.version}</version>
+    </dependency>
+

Review comment:
       (minor) remove the empty lines

##########
File path: 
pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerTest.java
##########
@@ -500,31 +515,32 @@ protected void stopFakeInstance(String instanceId) {
    */
   protected void addSchema(Schema schema)
       throws IOException {
-    String url = _controllerRequestURLBuilder.forSchemaCreate();
-    PostMethod postMethod = sendMultipartPostRequest(url, 
schema.toSingleLineJsonString());
-    assertEquals(postMethod.getStatusCode(), 200);
+    getControllerRequestClient().addSchema(schema);
   }
 
   protected Schema getSchema(String schemaName) {
-    Schema schema = _helixResourceManager.getSchema(schemaName);
-    assertNotNull(schema);
-    return schema;
+    try {

Review comment:
       Suggest not changing this one because all the getters are not using http 
request right now (e.g. `getOfflineTableConfig` and `getRealtimeTableConfig`). 
We may change them in a separate PR if necessary.

##########
File path: 
pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/minion/PinotTaskManagerStatelessTest.java
##########
@@ -163,6 +163,8 @@ public void testPinotTaskManagerSchedulerWithRestart()
     // Restart controller
     stopController();
     startController(properties);
+    // wait for controller to start correctly.
+    Thread.sleep(1000);

Review comment:
       Do you run into issues without this wait? We should avoid using this 
kind of wait to get instance ready as this can be flaky




-- 
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]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to