ayushtkn commented on code in PR #149:
URL: https://github.com/apache/tez/pull/149#discussion_r1349285875
##########
tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java:
##########
@@ -1056,6 +1057,21 @@ private void writePBTextFile(DAG dag) {
}
}
+ private void writePBJsonFile(DAGPlan dagPB, DAGImpl newDag) {
+ String logFile = logDirs[new Random().nextInt(logDirs.length)] +
File.separatorChar + newDag.getID() + "-"
Review Comment:
I think other files like ``TEZ_PB_PLAN_TEXT_NAME `` are in
``tezSysStagingPath``, should we keep these files also there?
https://github.com/apache/tez/blob/5bba1fffdd290ae3242615c23ab3b75c24f15e88/tez-api/src/main/java/org/apache/tez/common/TezCommonUtils.java#L211-L214
##########
tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java:
##########
@@ -1056,6 +1057,21 @@ private void writePBTextFile(DAG dag) {
}
}
+ private void writePBJsonFile(DAGPlan dagPB, DAGImpl newDag) {
+ String logFile = logDirs[new Random().nextInt(logDirs.length)] +
File.separatorChar + newDag.getID() + "-"
+ + TezConstants.TEZ_PB_PLAN_JSON_NAME;
+
+ LOG.info("Writing DAG JSON plan to: " + logFile);
Review Comment:
should use placeholders
```
LOG.info("Writing DAG JSON plan to: {}", logFile);
```
##########
tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java:
##########
@@ -1056,6 +1057,21 @@ private void writePBTextFile(DAG dag) {
}
}
+ private void writePBJsonFile(DAGPlan dagPB, DAGImpl newDag) {
+ String logFile = logDirs[new Random().nextInt(logDirs.length)] +
File.separatorChar + newDag.getID() + "-"
+ + TezConstants.TEZ_PB_PLAN_JSON_NAME;
+
+ LOG.info("Writing DAG JSON plan to: " + logFile);
+ File outFile = new File(logFile);
+ try {
+ PrintWriter printWriter = new PrintWriter(outFile, "UTF-8");
+ printWriter.println(DAGUtils.generateSimpleJSONPlan(dagPB));
+ printWriter.close();
Review Comment:
I think this close should be in ```finally``` block or we should use
try-with-resource for `printWriter`, else if ``
printWriter.println(DAGUtils.generateSimpleJSONPlan(dagPB));`` this throws
exception, we won't be closing the `printWritter`
##########
tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java:
##########
@@ -1056,6 +1057,21 @@ private void writePBTextFile(DAG dag) {
}
}
+ private void writePBJsonFile(DAGPlan dagPB, DAGImpl newDag) {
+ String logFile = logDirs[new Random().nextInt(logDirs.length)] +
File.separatorChar + newDag.getID() + "-"
+ + TezConstants.TEZ_PB_PLAN_JSON_NAME;
+
+ LOG.info("Writing DAG JSON plan to: " + logFile);
+ File outFile = new File(logFile);
+ try {
+ PrintWriter printWriter = new PrintWriter(outFile, "UTF-8");
+ printWriter.println(DAGUtils.generateSimpleJSONPlan(dagPB));
+ printWriter.close();
+ } catch (IOException | JSONException e) {
+ LOG.warn("Failed to write TEZ_PLAN JSON to " + outFile, e);
Review Comment:
should change to:
```
LOG.warn("Failed to write TEZ_PLAN JSON to {}", outFile, e);
```
--
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: issues-unsubscr...@tez.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
