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]