vinothchandar commented on pull request #2422:
URL: https://github.com/apache/hudi/pull/2422#issuecomment-757357033


   @satishkotha  This looks good and also much cleaner. 
   
   Only thing to do IMO is to make sure we don't throw an error when archival 
fails to delete the replaced file groups later.
   
   ```
    boolean deleteSuccess = deleteReplacedFileGroups(context, hoodieInstant);
           if (!deleteSuccess) {
             // throw error and stop archival if deleting replaced file groups 
failed.
             throw new HoodieCommitException("Unable to delete file(s) for " + 
hoodieInstant.getFileName());
           }
   ```
   
   I think we can can that code, if you are comfortable with this. Or just 
`log.WARN` instead of exception for now, with a follow on 0.8.0 JIRA to do 
that. 


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


Reply via email to