[
https://issues.apache.org/jira/browse/HADOOP-19189?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17852553#comment-17852553
]
ASF GitHub Bot commented on HADOOP-19189:
-----------------------------------------
mukund-thakur commented on code in PR #6857:
URL: https://github.com/apache/hadoop/pull/6857#discussion_r1628370772
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/commit/ITestS3ACommitterFactory.java:
##########
@@ -72,121 +85,141 @@ public class ITestS3ACommitterFactory extends
AbstractCommitITest {
* Parameterized list of bindings of committer name in config file to
* expected class instantiated.
*/
- private static final Object[][] bindings = {
- {COMMITTER_NAME_FILE, FileOutputCommitter.class},
- {COMMITTER_NAME_DIRECTORY, DirectoryStagingCommitter.class},
- {COMMITTER_NAME_PARTITIONED, PartitionedStagingCommitter.class},
- {InternalCommitterConstants.COMMITTER_NAME_STAGING,
- StagingCommitter.class},
- {COMMITTER_NAME_MAGIC, MagicS3GuardCommitter.class}
+ private static final Object[][] BINDINGS = {
Review Comment:
can you please explain the difference between 1st and 2nd param?
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/commit/ITestS3ACommitterFactory.java:
##########
@@ -72,121 +85,141 @@ public class ITestS3ACommitterFactory extends
AbstractCommitITest {
* Parameterized list of bindings of committer name in config file to
* expected class instantiated.
*/
- private static final Object[][] bindings = {
- {COMMITTER_NAME_FILE, FileOutputCommitter.class},
- {COMMITTER_NAME_DIRECTORY, DirectoryStagingCommitter.class},
- {COMMITTER_NAME_PARTITIONED, PartitionedStagingCommitter.class},
- {InternalCommitterConstants.COMMITTER_NAME_STAGING,
- StagingCommitter.class},
- {COMMITTER_NAME_MAGIC, MagicS3GuardCommitter.class}
+ private static final Object[][] BINDINGS = {
+ {"", "", FileOutputCommitter.class, "Default Binding"},
+ {COMMITTER_NAME_FILE, "", FileOutputCommitter.class, "File committer in
FS"},
+ {COMMITTER_NAME_PARTITIONED, "", PartitionedStagingCommitter.class,
+ "partitoned committer in FS"},
+ {COMMITTER_NAME_STAGING, "", StagingCommitter.class, "staging committer
in FS"},
+ {COMMITTER_NAME_MAGIC, "", MagicS3GuardCommitter.class, "magic committer
in FS"},
+ {COMMITTER_NAME_DIRECTORY, "", DirectoryStagingCommitter.class, "Dir
committer in FS"},
+ {INVALID_NAME, "", null, "invalid committer in FS"},
+
+ {"", COMMITTER_NAME_FILE, FileOutputCommitter.class, "File committer in
task"},
+ {"", COMMITTER_NAME_PARTITIONED, PartitionedStagingCommitter.class,
+ "partioned committer in task"},
+ {"", COMMITTER_NAME_STAGING, StagingCommitter.class, "staging committer
in task"},
+ {"", COMMITTER_NAME_MAGIC, MagicS3GuardCommitter.class, "magic committer
in task"},
+ {"", COMMITTER_NAME_DIRECTORY, DirectoryStagingCommitter.class, "Dir
committer in task"},
+ {"", INVALID_NAME, null, "invalid committer in task"},
};
/**
- * This is a ref to the FS conf, so changes here are visible
- * to callers querying the FS config.
+ * Test array for parameterized test runs.
+ *
+ * @return the committer binding for this run.
*/
- private Configuration filesystemConfRef;
-
- private Configuration taskConfRef;
-
- @Override
- public void setup() throws Exception {
- super.setup();
- jobId = randomJobId();
- attempt0 = "attempt_" + jobId + "_m_000000_0";
- taskAttempt0 = TaskAttemptID.forName(attempt0);
-
- outDir = path(getMethodName());
- factory = new S3ACommitterFactory();
- Configuration conf = new Configuration();
- conf.set(FileOutputFormat.OUTDIR, outDir.toUri().toString());
- conf.set(MRJobConfig.TASK_ATTEMPT_ID, attempt0);
- conf.setInt(MRJobConfig.APPLICATION_ATTEMPT_ID, 1);
- filesystemConfRef = getFileSystem().getConf();
- tContext = new TaskAttemptContextImpl(conf, taskAttempt0);
- taskConfRef = tContext.getConfiguration();
- }
-
- @Test
- public void testEverything() throws Throwable {
- testImplicitFileBinding();
- testBindingsInTask();
- testBindingsInFSConfig();
- testInvalidFileBinding();
- testInvalidTaskBinding();
+ @Parameterized.Parameters(name = "{3}-fs=[{0}]-task=[{1}]-[{2}]")
+ public static Collection<Object[]> params() {
+ return Arrays.asList(BINDINGS);
}
/**
- * Verify that if all config options are unset, the FileOutputCommitter
- *
- * is returned.
+ * Name of committer to set in fs config. If "" do not set one.
*/
- public void testImplicitFileBinding() throws Throwable {
- taskConfRef.unset(FS_S3A_COMMITTER_NAME);
- filesystemConfRef.unset(FS_S3A_COMMITTER_NAME);
- assertFactoryCreatesExpectedCommitter(FileOutputCommitter.class);
- }
+ private final String fsCommitterName;
Review Comment:
nit: some java doc,
> ITestS3ACommitterFactory failing
> --------------------------------
>
> Key: HADOOP-19189
> URL: https://issues.apache.org/jira/browse/HADOOP-19189
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: fs/s3, test
> Affects Versions: 3.4.0
> Reporter: Steve Loughran
> Priority: Minor
> Labels: pull-request-available
>
> we've had ITestS3ACommitterFactory failing for a while, where it looks like
> changed committer settings aren't being picked up.
> {code}
> ERROR]
> ITestS3ACommitterFactory.testEverything:115->testInvalidFileBinding:165
> Expected a org.apache.hadoop.fs.s3a.commit.PathCommitException to be thrown,
> but got the result: :
> FileOutputCommitter{PathOutputCommitter{context=TaskAttemptContextImpl{JobContextImpl
> {code}
> I've spent some time looking at it and it is happening because the test sets
> the fileystem ref for the local test fs, and not that of the filesystem
> created by the committer, which is where the option is picked up.
> i've tried to parameterize it but things are still playing up and I'm not
> sure how hard to try to fix.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
