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



##########
File path: 
pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/CreateSegmentCommand.java
##########
@@ -334,10 +333,10 @@ public void run() {
           for (int curr = 0; curr <= _retry; curr++) {
             try {
               SegmentGeneratorConfig config = new 
SegmentGeneratorConfig(segmentGeneratorConfig);
-
-              String localFile = dataFilePath.getName();
-              Path localFilePath = new Path(localFile);
-              dataDirPath.getFileSystem(new 
Configuration()).copyToLocalFile(dataFilePath, localFilePath);
+              URI dataFileUri = URI.create(dataFilePath);
+              String[] splits = dataFilePath.split("/");
+              String localFile = splits[splits.length - 1];

Review comment:
       Delete the local file after generating the segment? (Also recommend 
putting it into a temp dir)
   Also, not sure if it is a good idea directly using the file name because 
there might be conflict for nested dir.

##########
File path: 
pinot-tools/src/main/java/org/apache/pinot/tools/streams/githubevents/PullRequestMergedEventsStream.java
##########
@@ -206,41 +215,41 @@ public void start() {
    * Find commits, review comments, comments corresponding to this pull 
request event.
    * Construct a PullRequestMergedEvent with the help of the event, commits, 
review comments and comments.
    * Converts PullRequestMergedEvent to GenericRecord
+   * @param event
    */
-  private GenericRecord convertToPullRequestMergedGenericRecord(JsonElement 
eventJson)
+  private GenericRecord convertToPullRequestMergedGenericRecord(JsonNode event)
       throws IOException {
     GenericRecord genericRecord = null;
-    JsonObject event = eventJson.getAsJsonObject();
-    String type = event.get("type").getAsString();
+    String type = event.get("type").asText();
 
     if ("PullRequestEvent".equals(type)) {
-      JsonObject payload = event.get("payload").getAsJsonObject();
+      JsonNode payload = event.get("payload");
       if (payload != null) {
-        String action = payload.get("action").getAsString();
-        JsonObject pullRequest = payload.get("pull_request").getAsJsonObject();
-        String merged = pullRequest.get("merged").getAsString();
+        String action = payload.get("action").asText();
+        JsonNode pullRequest = payload.get("pull_request");
+        String merged = pullRequest.get("merged").asText();
         if ("closed".equals(action) && "true".equals(merged)) { // valid pull 
request merge event
 
-          JsonArray commits = null;
-          String commitsURL = pullRequest.get("commits_url").getAsString();
+          JsonNode commits = null;
+          String commitsURL = pullRequest.get("commits_url").asText();
           GitHubAPICaller.GitHubAPIResponse commitsResponse = 
_gitHubAPICaller.callAPI(commitsURL);
 
           if (commitsResponse.responseString != null) {
-            commits = new 
JsonParser().parse(commitsResponse.responseString).getAsJsonArray();
+            commits = 
OBJECT_MAPPER.reader().readTree(commitsResponse.responseString);

Review comment:
       ```suggestion
               commits = 
JsonUtils.stringToJsonNode(commitsResponse.responseString);
   ```
   Same for other places. No need to construct another object mapper




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

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