phet commented on code in PR #4047:
URL: https://github.com/apache/gobblin/pull/4047#discussion_r1744116119
##########
gobblin-temporal/src/main/java/org/apache/gobblin/temporal/ddm/activity/impl/GenerateWorkUnitsImpl.java:
##########
@@ -73,13 +77,22 @@ public int generateWorkUnits(Properties jobProps,
EventSubmitterContext eventSub
// before embarking on (potentially expensive) WU creation, first
pre-check that the FS is available
FileSystem fs = JobStateUtils.openFileSystem(jobState);
fs.mkdirs(workDirRoot);
-
- List<WorkUnit> workUnits = generateWorkUnitsForJobState(jobState,
eventSubmitterContext, closer);
-
+ boolean canCleanUpTempDirs = false; // unlike `AbstractJobLauncher`
running the job end-to-end, this is Work Discovery only, so WAY TOO SOON for
cleanup
+ DestinationDatasetHandlerService datasetHandlerService = closer.register(
+ new DestinationDatasetHandlerService(jobState, canCleanUpTempDirs,
eventSubmitterContext.create()));
+
+ List<WorkUnit> workUnits = generateWorkUnitsForJobState(jobState,
datasetHandlerService, closer);
+
+ // GET FOLDERS FROM HERE TO CLEANUP
+ Set<String> foldersToCleanup = new HashSet<>();
+ for (WorkUnit workUnit : workUnits) {
+
foldersToCleanup.add(workUnit.getProp(ConfigurationKeys.WRITER_STAGING_DIR));
+
foldersToCleanup.add(workUnit.getProp(ConfigurationKeys.WRITER_OUTPUT_DIR));
+ }
Review Comment:
let's abstract within a named method, as that is more easily tested. also,
don't we need to check `getProp` for when that's not found?
--
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]
