steveloughran commented on code in PR #5521:
URL: https://github.com/apache/hadoop/pull/5521#discussion_r1153027000
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AUrlScheme.java:
##########
@@ -0,0 +1,31 @@
+package org.apache.hadoop.fs.s3a;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.junit.Test;
+
+import java.io.IOException;
+
+public class ITestS3AUrlScheme extends AbstractS3ATestBase{
+
+ @Override
+ public void setup() throws Exception {
+ super.setup();
+ }
+
+ @Override
+ protected Configuration createConfiguration() {
+ Configuration conf = super.createConfiguration();
+ conf.set("fs.s3a.impl", "org.apache.hadoop.fs.s3a.S3AFileSystem");
Review Comment:
this is the default. why are you setting it again?
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AUrlScheme.java:
##########
@@ -0,0 +1,31 @@
+package org.apache.hadoop.fs.s3a;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.junit.Test;
+
+import java.io.IOException;
+
+public class ITestS3AUrlScheme extends AbstractS3ATestBase{
+
+ @Override
Review Comment:
not needed
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AUrlScheme.java:
##########
@@ -0,0 +1,31 @@
+package org.apache.hadoop.fs.s3a;
Review Comment:
yetus will reject this without the asf copyright header
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AUrlScheme.java:
##########
@@ -0,0 +1,31 @@
+package org.apache.hadoop.fs.s3a;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.junit.Test;
+
+import java.io.IOException;
+
+public class ITestS3AUrlScheme extends AbstractS3ATestBase{
+
+ @Override
+ public void setup() throws Exception {
+ super.setup();
+ }
+
+ @Override
+ protected Configuration createConfiguration() {
+ Configuration conf = super.createConfiguration();
+ conf.set("fs.s3a.impl", "org.apache.hadoop.fs.s3a.S3AFileSystem");
+ return conf;
+ }
+
+ @Test
+ public void testFSScheme() throws IOException {
+ FileSystem fs = getFileSystem();
+ assertEquals(fs.getScheme(), "s3a");
Review Comment:
1. junit assert equals has the order (expected, actual) for the error
messages to work
2. use AssertJ.assertThat() for new tests please -it's far more powerful.
while it takes a while to learn, smaller tests are the place to do it.
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AUrlScheme.java:
##########
@@ -0,0 +1,31 @@
+package org.apache.hadoop.fs.s3a;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.junit.Test;
+
+import java.io.IOException;
Review Comment:
check your import rules. they MUST be
```
java.*
java.*
--
not-asf.*
--
org.apace.*
--
statics
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3A.java:
##########
@@ -37,7 +37,8 @@ public class S3A extends DelegateToFileSystem {
public S3A(URI theUri, Configuration conf)
throws IOException, URISyntaxException {
- super(theUri, new S3AFileSystem(), conf, "s3a", false);
+ super(theUri, new S3AFileSystem(), conf,
+ theUri.getScheme().isEmpty() ? "s3a" : theUri.getScheme(), false);
Review Comment:
move this to a constant in `org.apache.hadoop.fs.s3a.impl.InternalConstants`
and then you can reference it in tests.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
