steveloughran commented on code in PR #6407:
URL: https://github.com/apache/hadoop/pull/6407#discussion_r1490936359
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/StoreContext.java:
##########
@@ -37,7 +37,7 @@
import org.apache.hadoop.fs.s3a.S3AFileStatus;
import org.apache.hadoop.fs.s3a.S3AInputPolicy;
import org.apache.hadoop.fs.s3a.S3AStorageStatistics;
-import org.apache.hadoop.fs.s3a.S3ObjectStorageClassFilter;
+import org.apache.hadoop.fs.s3a.api.S3ObjectStorageClassFilter;
Review Comment:
this has to move down to L42 now, as its in a sub package.
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/list/ITestS3AReadRestoredGlacierObjects.java:
##########
@@ -89,7 +92,7 @@ private FileSystem createFiles(String
s3ObjectStorageClassFilter) throws Throwab
FileSystem fs = contract.getTestFileSystem();
Path dir = methodPath();
fs.mkdirs(dir);
Review Comment:
skip this for a marginal increase in performance. create() assumes there's a
dir and just creates the object in the destination path without checks.
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java:
##########
@@ -52,6 +52,7 @@
import java.util.concurrent.atomic.AtomicBoolean;
import javax.annotation.Nullable;
+import org.apache.hadoop.fs.s3a.api.S3ObjectStorageClassFilter;
Review Comment:
same
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java:
##########
@@ -1525,6 +1526,11 @@ private Constants() {
*/
public static final String READ_RESTORED_GLACIER_OBJECTS =
"fs.s3a.glacier.read.restored.objects";
+ /**
+ * Default value of Read Restored Glacier objects config.
+ */
+ public static final String DEFAULT_READ_RESTORED_GLACIER_OBJECTS =
S3ObjectStorageClassFilter.READ_ALL.toString();
Review Comment:
split onto a new line to keep checkstyle happy
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Listing.java:
##########
@@ -18,6 +18,7 @@
package org.apache.hadoop.fs.s3a;
+import org.apache.hadoop.fs.s3a.api.S3ObjectStorageClassFilter;
Review Comment:
new org.apache imports MUST go into that block for new classes. set your IDE
up for this and life gets simpler for all
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/api/S3ObjectStorageClassFilter.java:
##########
@@ -16,8 +16,9 @@
* limitations under the License.
*/
-package org.apache.hadoop.fs.s3a;
+package org.apache.hadoop.fs.s3a.api;
+import org.apache.hadoop.fs.s3a.S3AFileSystem;
Review Comment:
now, this is a branch new class. in which case the L22 import can go into
the org.apache block and the S3aFilesystem one with it. the only reason it is
in the wrong place in existing code is that the move to repackaged classes was
a big search and replace only: no re-ordering
--
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]
