ujjawal4046 commented on code in PR #7177:
URL: https://github.com/apache/hbase/pull/7177#discussion_r2241521229


##########
hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TestTableOutputFormat.java:
##########
@@ -127,4 +128,21 @@ public void testTableOutputFormatWhenWalIsOFFForDelete()
     Assert.assertEquals("Durability of the mutation should be SKIP_WAL", 
Durability.SKIP_WAL,
       delete.getDurability());
   }
+
+  @Test
+  public void testOutputCommitterConfiguration() throws IOException, 
InterruptedException {
+    // 1. Verify it returns the default committer when the property is not set.
+    conf.unset(TableOutputFormat.OUTPUT_COMMITTER_CLASS);
+    tableOutputFormat.setConf(conf);
+    Assert.assertEquals("Should use default committer",
+      TableOutputCommitter.class,
+      tableOutputFormat.getOutputCommitter(context).getClass());
+
+    // 2. Verify it returns the custom committer when the property is set.
+    conf.set(TableOutputFormat.OUTPUT_COMMITTER_CLASS, 
CUSTOM_COMMITTER.getClass().getName());
+    tableOutputFormat.setConf(conf);
+    Assert.assertEquals("Should use custom committer",
+      CUSTOM_COMMITTER.getClass(),
+      tableOutputFormat.getOutputCommitter(context).getClass());

Review Comment:
   Can you create custom committer for the test since the class won't change 
for the CUSTOM_COMMITTER ? 



##########
hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/TableOutputFormat.java:
##########
@@ -252,7 +262,18 @@ public void checkOutputSpecs(JobContext context) throws 
IOException, Interrupted
   @Override
   public OutputCommitter getOutputCommitter(TaskAttemptContext context)
     throws IOException, InterruptedException {
-    return new TableOutputCommitter();
+    Configuration hConf = getConf();
+    if (hConf == null) {
+      hConf = context.getConfiguration();
+    }
+
+    try {
+      Class<? extends OutputCommitter> outputCommitter = 
hConf.getClass(OUTPUT_COMMITTER_CLASS,
+        TableOutputCommitter.class, OutputCommitter.class);
+      return ReflectionUtils.newInstance(outputCommitter);

Review Comment:
   Would the custom implementation for the output committer need any param ? 
The current reflection based implementation would call constructor with no 
params



-- 
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]

Reply via email to