You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/01/15 16:15:22 UTC
[GitHub] [ozone] symious opened a new pull request #1806: HDDS-2195. Apply spotbugs check to test code
symious opened a new pull request #1806:
URL: https://github.com/apache/ozone/pull/1806
## What changes were proposed in this pull request?
- Add spotbugs plugin.
- Fixed two bugs reported by spotbugs.
- Add other bugs reported by spotbugs to spotbugs-exclude.xml.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-2195
## How was this patch tested?
Manually test passed. Spotbugs plugin works fine by running "mvn compile site".
The compile is needed according to https://github.com/spotbugs/spotbugs-maven-plugin/issues/93.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] symious commented on pull request #1806: HDDS-2195. Apply spotbugs check to test code
Posted by GitBox <gi...@apache.org>.
symious commented on pull request #1806:
URL: https://github.com/apache/ozone/pull/1806#issuecomment-764662299
@adoroszlai Could you help to review the PR?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] symious commented on a change in pull request #1806: HDDS-2195. Apply spotbugs check to test code
Posted by GitBox <gi...@apache.org>.
symious commented on a change in pull request #1806:
URL: https://github.com/apache/ozone/pull/1806#discussion_r569118152
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/MiniOzoneHAClusterImpl.java
##########
@@ -250,7 +250,7 @@ public void stopOzoneManager(String omNodeId) {
*/
public static class Builder extends MiniOzoneClusterImpl.Builder {
- private final String nodeIdBaseStr = "omNode-";
+ private static final String NODEIDBASESTR = "omNode-";
Review comment:
Fixed.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] symious commented on pull request #1806: HDDS-2195. Apply spotbugs check to test code
Posted by GitBox <gi...@apache.org>.
symious commented on pull request #1806:
URL: https://github.com/apache/ozone/pull/1806#issuecomment-766187003
@adoroszlai Check green now, could you help to review the changes?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] symious commented on a change in pull request #1806: HDDS-2195. Apply spotbugs check to test code
Posted by GitBox <gi...@apache.org>.
symious commented on a change in pull request #1806:
URL: https://github.com/apache/ozone/pull/1806#discussion_r569184424
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractUnbufferTest.java
##########
@@ -84,7 +84,9 @@ public void testUnbufferOnClosedFile() throws IOException {
FSDataInputStream stream = null;
try {
stream = getFileSystem().open(file);
- validateFullFileContents(stream);
+ if (stream != null) {
+ validateFullFileContents(stream);
Review comment:
Added `assertNotNull` in `validateFileContents`.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on pull request #1806: HDDS-2195. Apply spotbugs check to test code
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1806:
URL: https://github.com/apache/ozone/pull/1806#issuecomment-771763277
> Will fix the issues later.
Sure, take your time. I'll try to help with resolving any merge conflicts.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] symious commented on a change in pull request #1806: HDDS-2195. Apply spotbugs check to test code
Posted by GitBox <gi...@apache.org>.
symious commented on a change in pull request #1806:
URL: https://github.com/apache/ozone/pull/1806#discussion_r569093385
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractUnbufferTest.java
##########
@@ -84,7 +84,9 @@ public void testUnbufferOnClosedFile() throws IOException {
FSDataInputStream stream = null;
try {
stream = getFileSystem().open(file);
- validateFullFileContents(stream);
+ if (stream != null) {
+ validateFullFileContents(stream);
Review comment:
Have excluded "RR_NOT_CHECKED" for integration-test, I think we can remove the isNull check?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] symious commented on pull request #1806: HDDS-2195. Apply spotbugs check to test code
Posted by GitBox <gi...@apache.org>.
symious commented on pull request #1806:
URL: https://github.com/apache/ozone/pull/1806#issuecomment-764662299
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] symious commented on pull request #1806: HDDS-2195. Apply spotbugs check to test code
Posted by GitBox <gi...@apache.org>.
symious commented on pull request #1806:
URL: https://github.com/apache/ozone/pull/1806#issuecomment-771758926
@adoroszlai Thanks for the detailed review. Will fix the issues later.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on a change in pull request #1806: HDDS-2195. Apply spotbugs check to test code
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #1806:
URL: https://github.com/apache/ozone/pull/1806#discussion_r568552958
##########
File path: hadoop-hdds/config/src/test/java/org/apache/hadoop/hdds/conf/TestConfigFileGenerator.java
##########
@@ -36,7 +36,8 @@
@Test
public void testGeneratedXml() throws FileNotFoundException {
String generatedXml =
- new Scanner(new File("target/test-classes/ozone-default-generated.xml"))
+ new Scanner(new File("target/test-classes/ozone-default-generated.xml"),
+ "UTF-8")
Review comment:
Please replace `"UTF-8"` with `StandardCharsets.UTF_8.name()`. Similarly in few other locations where this string is being added. This was recently improved in #1673, we should try to avoid regression.
##########
File path: hadoop-ozone/fault-injection-test/mini-chaos-tests/src/test/java/org/apache/hadoop/ozone/failure/FailureManager.java
##########
@@ -68,7 +68,7 @@ private void fail() {
f.fail(cluster);
} catch (Throwable t) {
LOG.info("Caught exception while inducing failure:{}", f.getName(), t);
- System.exit(-2);
+ throw new RuntimeException();
Review comment:
@mukul1987 will this keep existing behavior of chaos tests?
##########
File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/server/TestSCMBlockProtocolServer.java
##########
@@ -50,7 +50,7 @@
private StorageContainerManager scm;
private NodeManager nodeManager;
private ScmBlockLocationProtocolServerSideTranslatorPB service;
- private final int nodeCount = 10;
+ private static final int NODECOUNT = 10;
Review comment:
`NODE_COUNT`
##########
File path: hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/api/TestEndpoints.java
##########
@@ -133,27 +131,24 @@
private Pipeline pipeline;
private FileCountBySizeDao fileCountBySizeDao;
private DSLContext dslContext;
- private final String host1 = "host1.datanode";
- private final String host2 = "host2.datanode";
- private final String ip1 = "1.1.1.1";
- private final String ip2 = "2.2.2.2";
- private final String prometheusTestResponseFile =
+ private static final String HOST1 = "host1.datanode";
+ private static final String HOST2 = "host2.datanode";
+ private static final String IP1 = "1.1.1.1";
+ private static final String IP2 = "2.2.2.2";
+ private static final String PROMETHEUSTESTRESPONSEFILE =
Review comment:
`PROMETHEUS_TEST_RESPONSE_FILE`
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractUnbufferTest.java
##########
@@ -84,7 +84,9 @@ public void testUnbufferOnClosedFile() throws IOException {
FSDataInputStream stream = null;
try {
stream = getFileSystem().open(file);
- validateFullFileContents(stream);
+ if (stream != null) {
+ validateFullFileContents(stream);
Review comment:
Instead of the null check, can we add an `assertNotNull` in `validateFileContents`?
##########
File path: hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/security/TestOMGetDelegationTokenRequest.java
##########
@@ -56,7 +56,7 @@
private OMRequest originalRequest;
private OMRequest modifiedRequest;
private OMGetDelegationTokenRequest omGetDelegationTokenRequest;
- private final String checkResponse = "";
+ private static final String CHECKRESPONSE = "";
Review comment:
`CHECK_RESPONSE`
##########
File path: hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyRequest.java
##########
@@ -78,8 +79,8 @@
protected OzoneBlockTokenSecretManager ozoneBlockTokenSecretManager;
protected ScmBlockLocationProtocol scmBlockLocationProtocol;
- protected final long containerID = 1000L;
- protected final long localID = 100L;
+ protected static final long CONTAINERID = 1000L;
+ protected static final long LOCALID = 100L;
Review comment:
`CONTAINER_ID` and `LOCAL_ID` would be more in line with naming conventions.
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneClientRetriesOnException.java
##########
@@ -62,7 +62,7 @@
/**
* Tests failure detection and handling in BlockOutputStream Class.
*/
-public class TestOzoneClientRetriesOnException {
+public class TestOzoneClientRetriesOnException extends Exception {
Review comment:
Seems to be a false positive. `TestOzoneClientRetriesOnException` means this class tests that the client does retry when it encounters exceptions. I think it could be renamed to `TestOzoneClientRetriesOnExceptions` to satisfy spotbugs.
##########
File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementFactory.java
##########
@@ -55,7 +55,7 @@
// datanodes array list
private List<DatanodeDetails> datanodes = new ArrayList<>();
// node storage capacity
- private final long storageCapacity = 100L;
+ private static final long STORAGECAPACITY = 100L;
Review comment:
`STORAGE_CAPACITY`
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/MiniOzoneHAClusterImpl.java
##########
@@ -250,7 +250,7 @@ public void stopOzoneManager(String omNodeId) {
*/
public static class Builder extends MiniOzoneClusterImpl.Builder {
- private final String nodeIdBaseStr = "omNode-";
+ private static final String NODEIDBASESTR = "omNode-";
Review comment:
How about `NODE_ID_PREFIX`?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] symious commented on a change in pull request #1806: HDDS-2195. Apply spotbugs check to test code
Posted by GitBox <gi...@apache.org>.
symious commented on a change in pull request #1806:
URL: https://github.com/apache/ozone/pull/1806#discussion_r569118342
##########
File path: hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/security/TestOMGetDelegationTokenRequest.java
##########
@@ -56,7 +56,7 @@
private OMRequest originalRequest;
private OMRequest modifiedRequest;
private OMGetDelegationTokenRequest omGetDelegationTokenRequest;
- private final String checkResponse = "";
+ private static final String CHECKRESPONSE = "";
Review comment:
Fixed.
##########
File path: hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyRequest.java
##########
@@ -78,8 +79,8 @@
protected OzoneBlockTokenSecretManager ozoneBlockTokenSecretManager;
protected ScmBlockLocationProtocol scmBlockLocationProtocol;
- protected final long containerID = 1000L;
- protected final long localID = 100L;
+ protected static final long CONTAINERID = 1000L;
+ protected static final long LOCALID = 100L;
Review comment:
Fixed.
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneClientRetriesOnException.java
##########
@@ -62,7 +62,7 @@
/**
* Tests failure detection and handling in BlockOutputStream Class.
*/
-public class TestOzoneClientRetriesOnException {
+public class TestOzoneClientRetriesOnException extends Exception {
Review comment:
Fixed.
##########
File path: hadoop-hdds/config/src/test/java/org/apache/hadoop/hdds/conf/TestConfigFileGenerator.java
##########
@@ -36,7 +36,8 @@
@Test
public void testGeneratedXml() throws FileNotFoundException {
String generatedXml =
- new Scanner(new File("target/test-classes/ozone-default-generated.xml"))
+ new Scanner(new File("target/test-classes/ozone-default-generated.xml"),
+ "UTF-8")
Review comment:
Fixed.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] symious commented on pull request #1806: HDDS-2195. Apply spotbugs check to test code
Posted by GitBox <gi...@apache.org>.
symious commented on pull request #1806:
URL: https://github.com/apache/ozone/pull/1806#issuecomment-769161647
Understand. @adoroszlai
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on pull request #1806: HDDS-2195. Apply spotbugs check to test code
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1806:
URL: https://github.com/apache/ozone/pull/1806#issuecomment-773059777
Thanks @symious for the contribution!
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] symious commented on pull request #1806: HDDS-2195. Apply spotbugs check to test code
Posted by GitBox <gi...@apache.org>.
symious commented on pull request #1806:
URL: https://github.com/apache/ozone/pull/1806#issuecomment-765893894
Still have some findbugs and checkstyle to fix.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] symious commented on pull request #1806: HDDS-2195. Apply spotbugs check to test code
Posted by GitBox <gi...@apache.org>.
symious commented on pull request #1806:
URL: https://github.com/apache/ozone/pull/1806#issuecomment-772514816
@adoroszlai Thanks for the review.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] symious commented on pull request #1806: HDDS-2195. Apply spotbugs check to test code
Posted by GitBox <gi...@apache.org>.
symious commented on pull request #1806:
URL: https://github.com/apache/ozone/pull/1806#issuecomment-765884781
@adoroszlai Please have a check on the commits, passed the findbugs check and tests locally.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai merged pull request #1806: HDDS-2195. Apply spotbugs check to test code
Posted by GitBox <gi...@apache.org>.
adoroszlai merged pull request #1806:
URL: https://github.com/apache/ozone/pull/1806
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] symious commented on pull request #1806: HDDS-2195. Apply spotbugs check to test code
Posted by GitBox <gi...@apache.org>.
symious commented on pull request #1806:
URL: https://github.com/apache/ozone/pull/1806#issuecomment-765003974
@adoroszlai Thanks for the detailed comment, I will fix the patch later.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on a change in pull request #1806: HDDS-2195. Apply spotbugs check to test code
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #1806:
URL: https://github.com/apache/ozone/pull/1806#discussion_r568552958
##########
File path: hadoop-hdds/config/src/test/java/org/apache/hadoop/hdds/conf/TestConfigFileGenerator.java
##########
@@ -36,7 +36,8 @@
@Test
public void testGeneratedXml() throws FileNotFoundException {
String generatedXml =
- new Scanner(new File("target/test-classes/ozone-default-generated.xml"))
+ new Scanner(new File("target/test-classes/ozone-default-generated.xml"),
+ "UTF-8")
Review comment:
Please replace `"UTF-8"` with `StandardCharsets.UTF_8.name()`. Similarly in few other locations where this string is being added. This was recently improved in #1673, we should try to avoid regression.
##########
File path: hadoop-ozone/fault-injection-test/mini-chaos-tests/src/test/java/org/apache/hadoop/ozone/failure/FailureManager.java
##########
@@ -68,7 +68,7 @@ private void fail() {
f.fail(cluster);
} catch (Throwable t) {
LOG.info("Caught exception while inducing failure:{}", f.getName(), t);
- System.exit(-2);
+ throw new RuntimeException();
Review comment:
@mukul1987 will this keep existing behavior of chaos tests?
##########
File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/server/TestSCMBlockProtocolServer.java
##########
@@ -50,7 +50,7 @@
private StorageContainerManager scm;
private NodeManager nodeManager;
private ScmBlockLocationProtocolServerSideTranslatorPB service;
- private final int nodeCount = 10;
+ private static final int NODECOUNT = 10;
Review comment:
`NODE_COUNT`
##########
File path: hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/api/TestEndpoints.java
##########
@@ -133,27 +131,24 @@
private Pipeline pipeline;
private FileCountBySizeDao fileCountBySizeDao;
private DSLContext dslContext;
- private final String host1 = "host1.datanode";
- private final String host2 = "host2.datanode";
- private final String ip1 = "1.1.1.1";
- private final String ip2 = "2.2.2.2";
- private final String prometheusTestResponseFile =
+ private static final String HOST1 = "host1.datanode";
+ private static final String HOST2 = "host2.datanode";
+ private static final String IP1 = "1.1.1.1";
+ private static final String IP2 = "2.2.2.2";
+ private static final String PROMETHEUSTESTRESPONSEFILE =
Review comment:
`PROMETHEUS_TEST_RESPONSE_FILE`
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractUnbufferTest.java
##########
@@ -84,7 +84,9 @@ public void testUnbufferOnClosedFile() throws IOException {
FSDataInputStream stream = null;
try {
stream = getFileSystem().open(file);
- validateFullFileContents(stream);
+ if (stream != null) {
+ validateFullFileContents(stream);
Review comment:
Instead of the null check, can we add an `assertNotNull` in `validateFileContents`?
##########
File path: hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/security/TestOMGetDelegationTokenRequest.java
##########
@@ -56,7 +56,7 @@
private OMRequest originalRequest;
private OMRequest modifiedRequest;
private OMGetDelegationTokenRequest omGetDelegationTokenRequest;
- private final String checkResponse = "";
+ private static final String CHECKRESPONSE = "";
Review comment:
`CHECK_RESPONSE`
##########
File path: hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyRequest.java
##########
@@ -78,8 +79,8 @@
protected OzoneBlockTokenSecretManager ozoneBlockTokenSecretManager;
protected ScmBlockLocationProtocol scmBlockLocationProtocol;
- protected final long containerID = 1000L;
- protected final long localID = 100L;
+ protected static final long CONTAINERID = 1000L;
+ protected static final long LOCALID = 100L;
Review comment:
`CONTAINER_ID` and `LOCAL_ID` would be more in line with naming conventions.
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneClientRetriesOnException.java
##########
@@ -62,7 +62,7 @@
/**
* Tests failure detection and handling in BlockOutputStream Class.
*/
-public class TestOzoneClientRetriesOnException {
+public class TestOzoneClientRetriesOnException extends Exception {
Review comment:
Seems to be a false positive. `TestOzoneClientRetriesOnException` means this class tests that the client does retry when it encounters exceptions. I think it could be renamed to `TestOzoneClientRetriesOnExceptions` to satisfy spotbugs.
##########
File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementFactory.java
##########
@@ -55,7 +55,7 @@
// datanodes array list
private List<DatanodeDetails> datanodes = new ArrayList<>();
// node storage capacity
- private final long storageCapacity = 100L;
+ private static final long STORAGECAPACITY = 100L;
Review comment:
`STORAGE_CAPACITY`
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/MiniOzoneHAClusterImpl.java
##########
@@ -250,7 +250,7 @@ public void stopOzoneManager(String omNodeId) {
*/
public static class Builder extends MiniOzoneClusterImpl.Builder {
- private final String nodeIdBaseStr = "omNode-";
+ private static final String NODEIDBASESTR = "omNode-";
Review comment:
How about `NODE_ID_PREFIX`?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] symious commented on pull request #1806: HDDS-2195. Apply spotbugs check to test code
Posted by GitBox <gi...@apache.org>.
symious commented on pull request #1806:
URL: https://github.com/apache/ozone/pull/1806#issuecomment-771758926
@adoroszlai Thanks for the detailed review. Will fix the issues later.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] symious commented on a change in pull request #1806: HDDS-2195. Apply spotbugs check to test code
Posted by GitBox <gi...@apache.org>.
symious commented on a change in pull request #1806:
URL: https://github.com/apache/ozone/pull/1806#discussion_r569118214
##########
File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/server/TestSCMBlockProtocolServer.java
##########
@@ -50,7 +50,7 @@
private StorageContainerManager scm;
private NodeManager nodeManager;
private ScmBlockLocationProtocolServerSideTranslatorPB service;
- private final int nodeCount = 10;
+ private static final int NODECOUNT = 10;
Review comment:
Fixed.
##########
File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementFactory.java
##########
@@ -55,7 +55,7 @@
// datanodes array list
private List<DatanodeDetails> datanodes = new ArrayList<>();
// node storage capacity
- private final long storageCapacity = 100L;
+ private static final long STORAGECAPACITY = 100L;
Review comment:
Fixed.
##########
File path: hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/api/TestEndpoints.java
##########
@@ -133,27 +131,24 @@
private Pipeline pipeline;
private FileCountBySizeDao fileCountBySizeDao;
private DSLContext dslContext;
- private final String host1 = "host1.datanode";
- private final String host2 = "host2.datanode";
- private final String ip1 = "1.1.1.1";
- private final String ip2 = "2.2.2.2";
- private final String prometheusTestResponseFile =
+ private static final String HOST1 = "host1.datanode";
+ private static final String HOST2 = "host2.datanode";
+ private static final String IP1 = "1.1.1.1";
+ private static final String IP2 = "2.2.2.2";
+ private static final String PROMETHEUSTESTRESPONSEFILE =
Review comment:
Fixed.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on pull request #1806: HDDS-2195. Apply spotbugs check to test code
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1806:
URL: https://github.com/apache/ozone/pull/1806#issuecomment-771763277
> Will fix the issues later.
Sure, take your time. I'll try to help with resolving any merge conflicts.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org