Skip to content

HADOOP-16637. Fix findbugs warnings in hadoop-cos. #1640

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from

Conversation

cxorm
Copy link
Member

@cxorm cxorm commented Oct 10, 2019

NOTICE

Please create an issue in ASF JIRA before opening a pull request,
and you need to set the title of the pull request which starts with
the corresponding JIRA issue number. (e.g. HADOOP-XXXXX. Fix a typo in YYY.)
For more details, please see https://cwiki.apache.org/confluence/display/HADOOP/How+To+Contribute

@hadoop-yetus
Copy link

💔 -1 overall

Vote Subsystem Runtime Comment
0 reexec 81 Docker mode activated.
_ Prechecks _
+1 dupname 0 No case conflicting files found.
+1 @author 0 The patch does not contain any @author tags.
-1 test4tests 0 The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ trunk Compile Tests _
+1 mvninstall 1287 trunk passed
+1 compile 19 trunk passed
+1 checkstyle 17 trunk passed
+1 mvnsite 21 trunk passed
+1 shadedclient 848 branch has no errors when building and testing our client artifacts.
+1 javadoc 20 trunk passed
0 spotbugs 33 Used deprecated FindBugs config; considering switching to SpotBugs.
-1 findbugs 31 hadoop-cloud-storage-project/hadoop-cos in trunk has 5 extant findbugs warnings.
_ Patch Compile Tests _
+1 mvninstall 19 the patch passed
+1 compile 14 the patch passed
+1 javac 14 the patch passed
-0 checkstyle 13 hadoop-cloud-storage-project/hadoop-cos: The patch generated 18 new + 0 unchanged - 0 fixed = 18 total (was 0)
+1 mvnsite 17 the patch passed
+1 whitespace 0 The patch has no whitespace issues.
+1 shadedclient 863 patch has no errors when building and testing our client artifacts.
+1 javadoc 18 the patch passed
+1 findbugs 38 hadoop-cloud-storage-project/hadoop-cos generated 0 new + 0 unchanged - 5 fixed = 0 total (was 5)
_ Other Tests _
+1 unit 18 hadoop-cos in the patch passed.
+1 asflicense 30 The patch does not generate ASF License warnings.
3469
Subsystem Report/Notes
Docker Client=19.03.3 Server=19.03.3 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1640/1/artifact/out/Dockerfile
GITHUB PR #1640
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
uname Linux 0f0f8b258186 4.15.0-54-generic #58-Ubuntu SMP Mon Jun 24 10:55:24 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality personality/hadoop.sh
git revision trunk / a031388
Default Java 1.8.0_222
findbugs https://builds.apache.org/job/hadoop-multibranch/job/PR-1640/1/artifact/out/branch-findbugs-hadoop-cloud-storage-project_hadoop-cos-warnings.html
checkstyle https://builds.apache.org/job/hadoop-multibranch/job/PR-1640/1/artifact/out/diff-checkstyle-hadoop-cloud-storage-project_hadoop-cos.txt
Test Results https://builds.apache.org/job/hadoop-multibranch/job/PR-1640/1/testReport/
Max. process+thread count 306 (vs. ulimit of 5500)
modules C: hadoop-cloud-storage-project/hadoop-cos U: hadoop-cloud-storage-project/hadoop-cos
Console output https://builds.apache.org/job/hadoop-multibranch/job/PR-1640/1/console
versions git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1
Powered by Apache Yetus 0.10.0 http://yetus.apache.org

This message was automatically generated.

@@ -249,7 +250,15 @@ public void storeEmptyFile(String key) throws IOException {

public PartETag uploadPart(File file, String key, String uploadId,
int partNum) throws IOException {
InputStream inputStream = new FileInputStream(file);
InputStream inputStream = null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks broken, as L262 will always get a closed stream

@steveloughran
Copy link
Contributor

Most of the changes look good; that input stream cleanup looks like it will break the code as a closed stream will always get passed along to the next method. The findbugs file probably just needs changing.

Which endpoint did you run the tests against?

@cxorm
Copy link
Member Author

cxorm commented Oct 23, 2019

Most of the changes look good; that input stream cleanup looks like it will break the code as a closed stream will always get passed along to the next method. The findbugs file probably just needs changing.

Which endpoint did you run the tests against?

We just ran the findbugs for the file.

@steveloughran
Copy link
Contributor

ok. You need to run the full test suite against a live service and declare which location/endpoint you tested with

that's the policy for all object store patches I'm afraid -Yetus can't be given the credentials and yet we need to stop regressions creeping in. Sorry

@cxorm
Copy link
Member Author

cxorm commented Oct 28, 2019

Thanks @steveloughran for the comment.
I'm going to run the related tests.

} else {
throw new IOException("creating buffer dir: " + dir.getAbsolutePath()
+ "unsuccessfully.");
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you fix indent?

Copy link
Member Author

@cxorm cxorm Jan 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @aajisaka for the review.
Yes I would fix it, and run the test on this issue.

@aajisaka
Copy link
Member

Hi @cxorm, how is this issue going?

@cxorm
Copy link
Member Author

cxorm commented Mar 13, 2020

Thanks @aajisaka for the review.
And we test the patch with our cluster for at least 30 days without any error.

@hadoop-yetus
Copy link

💔 -1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 24m 36s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
-1 ❌ test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ trunk Compile Tests _
+1 💚 mvninstall 19m 26s trunk passed
+1 💚 compile 0m 24s trunk passed
+1 💚 checkstyle 0m 21s trunk passed
+1 💚 mvnsite 0m 26s trunk passed
+1 💚 shadedclient 14m 53s branch has no errors when building and testing our client artifacts.
+1 💚 javadoc 0m 25s trunk passed
+0 🆗 spotbugs 0m 38s Used deprecated FindBugs config; considering switching to SpotBugs.
-1 ❌ findbugs 0m 36s hadoop-cloud-storage-project/hadoop-cos in trunk has 5 extant findbugs warnings.
_ Patch Compile Tests _
+1 💚 mvninstall 0m 21s the patch passed
+1 💚 compile 0m 16s the patch passed
+1 💚 javac 0m 16s the patch passed
-0 ⚠️ checkstyle 0m 13s hadoop-cloud-storage-project/hadoop-cos: The patch generated 3 new + 0 unchanged - 0 fixed = 3 total (was 0)
+1 💚 mvnsite 0m 18s the patch passed
+1 💚 whitespace 0m 0s The patch has no whitespace issues.
+1 💚 shadedclient 13m 40s patch has no errors when building and testing our client artifacts.
+1 💚 javadoc 0m 21s the patch passed
+1 💚 findbugs 0m 40s hadoop-cloud-storage-project/hadoop-cos generated 0 new + 1 unchanged - 4 fixed = 1 total (was 5)
_ Other Tests _
+1 💚 unit 0m 19s hadoop-cos in the patch passed.
+1 💚 asflicense 0m 31s The patch does not generate ASF License warnings.
79m 16s
Subsystem Report/Notes
Docker Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1640/2/artifact/out/Dockerfile
GITHUB PR #1640
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
uname Linux 35751b260277 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality personality/hadoop.sh
git revision trunk / 3d5ade1
Default Java 1.8.0_242
findbugs https://builds.apache.org/job/hadoop-multibranch/job/PR-1640/2/artifact/out/branch-findbugs-hadoop-cloud-storage-project_hadoop-cos-warnings.html
checkstyle https://builds.apache.org/job/hadoop-multibranch/job/PR-1640/2/artifact/out/diff-checkstyle-hadoop-cloud-storage-project_hadoop-cos.txt
Test Results https://builds.apache.org/job/hadoop-multibranch/job/PR-1640/2/testReport/
Max. process+thread count 414 (vs. ulimit of 5500)
modules C: hadoop-cloud-storage-project/hadoop-cos U: hadoop-cloud-storage-project/hadoop-cos
Console output https://builds.apache.org/job/hadoop-multibranch/job/PR-1640/2/console
versions git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1
Powered by Apache Yetus 0.11.1 https://yetus.apache.org

This message was automatically generated.

@aajisaka
Copy link
Member

@cxorm cxorm closed this Oct 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants