sv2000 commented on a change in pull request #3158:
URL: https://github.com/apache/gobblin/pull/3158#discussion_r595348947
##########
File path:
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/AbstractJobLauncher.java
##########
@@ -912,13 +915,14 @@ private void cleanLeftoverStagingData(WorkUnitStream
workUnits, JobState jobStat
}
try {
- if (this.jobContext.shouldCleanupStagingDataPerTask()) {
+ if (this.jobContext.shouldCleanupStagingDataPerTask() ||
jobState.getPropAsBoolean(ConfigurationKeys.USE_DATASET_LOCAL_WORK_DIR)) {
Review comment:
Why do we need to change the condition in the if (..) statement?
##########
File path:
gobblin-utility/src/main/java/org/apache/gobblin/util/JobLauncherUtils.java
##########
@@ -235,6 +245,14 @@ public static void cleanTaskStagingData(State state,
Logger logger, Closer close
logger.info("Cleaning up output directory " +
outputPath.toUri().getPath());
parallelRunner.deletePath(outputPath, true);
}
+
+ if (state.contains(ConfigurationKeys.ROW_LEVEL_ERR_FILE) &&
state.getPropAsBoolean(ConfigurationKeys.USE_DATASET_LOCAL_WORK_DIR, false)) {
Review comment:
same comment as earlier.
##########
File path:
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/AbstractJobLauncher.java
##########
@@ -972,7 +974,7 @@ private void cleanupStagingData(JobState jobState)
throw new JobException("Failed to check unfinished commit sequences", e);
}
- if (this.jobContext.shouldCleanupStagingDataPerTask()) {
+ if (this.jobContext.shouldCleanupStagingDataPerTask() ||
jobState.getPropAsBoolean(ConfigurationKeys.USE_DATASET_LOCAL_WORK_DIR)) {
Review comment:
Same comment as above.
##########
File path:
gobblin-utility/src/main/java/org/apache/gobblin/util/JobLauncherUtils.java
##########
@@ -197,6 +197,16 @@ public static void cleanTaskStagingData(State state,
Logger logger) throws IOExc
throw new IOException("Clean up output directory " +
outputPath.toUri().getPath() + " failed");
}
}
+
+ if (state.contains(ConfigurationKeys.ROW_LEVEL_ERR_FILE) &&
state.getPropAsBoolean(ConfigurationKeys.USE_DATASET_LOCAL_WORK_DIR, false)) {
Review comment:
We should avoid leaking details about dataset-specific local work dir
all over the place. This change seems unnecessary too.
----------------------------------------------------------------
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]
