[GitHub] [hadoop] steveloughran commented on issue #1761: HADOOP-16759. Filesystem openFile() builder to take a FileStatus param
steveloughran commented on issue #1761: HADOOP-16759. Filesystem openFile() builder to take a FileStatus param URL: https://github.com/apache/hadoop/pull/1761#issuecomment-576429973 Latest iteration -tested s3 ireland *Address the checkstyle issues. * Adds some tests of the API in ITestS3GuardOutOfBandOperations, for auth and non-auth. This includes the discovery (and fix!) of the fact that with the specific s3guard retry logic of HADOOP-16490, we needed to set a different retry option to get the tests to fail fast on deleted file in auth mode. It has been like that for a few months, but we've never noticed...though even in parallel runs it would have reduced performance by using up a process for 2+ minutes. Running in the IDE I initially thought my changes had broken something. Also: ITestS3Select fails as trying to select a dir raises an FNFE, just as open() always has. Because we skip looking for dir markers in select or open, attempts to read a nonexistent file will fail faster (though still add a 404 to the S3 cache) 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 With regards, Apache Git Services - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on issue #1761: HADOOP-16759. Filesystem openFile() builder to take a FileStatus param
steveloughran commented on issue #1761: HADOOP-16759. Filesystem openFile() builder to take a FileStatus param URL: https://github.com/apache/hadoop/pull/1761#issuecomment-573054318 style ``` ./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/AbstractFileSystem.java:34:import java.util.Set;:8: Unused import - java.util.Set. [UnusedImports] ./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ChecksumFileSystem.java:853: new CompletableFuture<>(), () -> open(path, parameters.getBufferSize()));: Line is longer than 80 characters (found 81). [LineLength] ./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/DelegateToFileSystem.java:269: * {@link FileSystem#openFileWithOptions(Path, org.apache.hadoop.fs.impl.OpenFileParameters)}.: Line is longer than 80 characters (found 96). [LineLength] ./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java:4615: .withStatus(super.getStatus()); // explicit so as to avoid IDE warnings: Line is longer than 80 characters (found 82). [LineLength] ./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/impl/FutureDataInputStreamBuilderImpl.java:150: public FutureDataInputStreamBuilder withFileStatus(FileStatus status) {:65: 'status' hides a field. [HiddenField] ./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/impl/FutureDataInputStreamBuilderImpl.java:155: /**: First sentence should end with a period. [JavadocStyle] ./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/impl/OpenFileParameters.java:59: public OpenFileParameters withMandatoryKeys(final Set mandatoryKeys) {:65: 'mandatoryKeys' hides a field. [HiddenField] ./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/impl/OpenFileParameters.java:64: public OpenFileParameters withOptions(final Configuration options) {:61: 'options' hides a field. [HiddenField] ./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/impl/OpenFileParameters.java:69: public OpenFileParameters withBufferSize(final int bufferSize) {:54: 'bufferSize' hides a field. [HiddenField] ./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/impl/OpenFileParameters.java:74: public OpenFileParameters withStatus(final FileStatus status) {:57: 'status' hides a field. [HiddenField] ./hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java:1004: S3AFileStatus fileStatus = extractOrFetchSimpleFileStatus(path, providedStatus);: Line is longer than 80 characters (found 84). [LineLength] ./hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java:4322: final S3AFileStatus fileStatus = extractOrFetchSimpleFileStatus(path, providedStatus);: Line is longer than 80 characters (found 90). [LineLength] ./hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3ARemoteFileChanged.java:472: || changeDetectionSource.equals(CHANGE_DETECT_SOURCE_VERSION_ID));: 'method call' child has incorrect indentation level 8, expected level should be 10. [Indentation] ./hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3ARemoteFileChanged.java:485: instream.read();: 'try' child has incorrect indentation level 7, expected level should be 6. [Indentation] ./hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3ARemoteFileChanged.java:520: instream.read();: 'if' child has incorrect indentation level 12, expected level should be 8. [Indentation] ./hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3ARemoteFileChanged.java:521: } else {: 'if rcurly' has incorrect indentation level 10, expected level should be 6. [Indentation] ./hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3ARemoteFileChanged.java:1012: S3AFileStatus newStatus = writeFile(testpath, dataset, dataset.length / 2, true);: Line is longer than 80 characters (found 85). [LineLength] ``` 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 With regards, Apache Git Services - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on issue #1761: HADOOP-16759. Filesystem openFile() builder to take a FileStatus param
steveloughran commented on issue #1761: HADOOP-16759. Filesystem openFile() builder to take a FileStatus param URL: https://github.com/apache/hadoop/pull/1761#issuecomment-572678918 update with the recommended changes, and some extra tests I could think of. Sorry, but I rebased the branch while I wasn't paying full attention -things aren't going to match up properly. I'm going to do that again once I've done the auth mode merge as I want as much QE of that patch as I can 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 With regards, Apache Git Services - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on issue #1761: HADOOP-16759. Filesystem openFile() builder to take a FileStatus param
steveloughran commented on issue #1761: HADOOP-16759. Filesystem openFile() builder to take a FileStatus param URL: https://github.com/apache/hadoop/pull/1761#issuecomment-566048209 thanks for the comments. I'm on vacation until jan so will address them then. 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 With regards, Apache Git Services - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on issue #1761: HADOOP-16759. Filesystem openFile() builder to take a FileStatus param
steveloughran commented on issue #1761: HADOOP-16759. Filesystem openFile() builder to take a FileStatus param URL: https://github.com/apache/hadoop/pull/1761#issuecomment-565430013 tested -s3a ireland. Not tested the other stores to make sure they don't break (as they don't read the status, it's hard to see how). Could also add more failure tests (path mismatch, ...) and use s3a metrics to verify HEAD doesn't count 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 With regards, Apache Git Services - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org