[GitHub] [incubator-gobblin] ZihanLi58 commented on a change in pull request #3078: [GOBBLIN-1233] Add case-aware support in WhitelistBlacklist and other small fixes

[GitBox <gi...@apache.org>]

ZihanLi58 commented on a change in pull request #3078:
URL: https://github.com/apache/incubator-gobblin/pull/3078#discussion_r470772296



##########
File path: gobblin-modules/gobblin-http/src/main/java/org/apache/gobblin/writer/AsyncHttpWriter.java
##########
@@ -83,7 +83,7 @@ protected void dispatch(Queue<BufferedRecord<D>> buffer) throws DispatchExceptio
     while (attempt < maxAttempts) {
       try {
           response = httpClient.sendRequest(rawRequest);
-      } catch (IOException e) {
+      } catch (Exception e) {

Review comment:
       Why we want change IOException to a more general Exception here?

##########
File path: gobblin-data-management/src/main/java/org/apache/gobblin/runtime/retention/DatasetCleanerTask.java
##########
@@ -47,8 +51,15 @@ public void run() {
       DatasetCleaner datasetCleaner = new DatasetCleaner(FileSystem.get(new Configuration()),
           this.taskContext.getTaskState().getProperties());
       datasetCleaner.clean();
+      this.workingState = WorkUnitState.WorkingState.SUCCESSFUL;
     } catch (IOException e) {
+      this.workingState = WorkUnitState.WorkingState.FAILED;
       throw new RuntimeException(e);
     }
   }
+
+  @Override
+  public void commit() {
+    log.info("task {} commits with state {}", this.taskContext.getTaskState().getTaskId(), this.workingState);

Review comment:
       Just curious, if this task runs in mapper or driver? I think most gobblin retention job runs on driver, if that's the case, what's the taskID here?




----------------------------------------------------------------
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:
users@infra.apache.org