You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@bookkeeper.apache.org by GitBox <gi...@apache.org> on 2022/03/30 16:59:35 UTC

[GitHub] [bookkeeper] nicoloboschi opened a new pull request #3160: [build] Fix spotbugs on JDK8

nicoloboschi opened a new pull request #3160:
URL: https://github.com/apache/bookkeeper/pull/3160


   ### Motivation
   If you run spotbugs with JDK8 it fails on bookkeper-server in this way:
   >Error:  Exception is caught when Exception is not thrown in org.apache.bookkeeper.proto.checksum.DirectMemoryCRC32Digest.<static initializer for DirectMemoryCRC32Digest>() [org.apache.bookkeeper.proto.checksum.DirectMemoryCRC32Digest] At DirectMemoryCRC32Digest.java:[line 86] REC_CATCH_EXCEPTION
   
   I don't think it makes much sense. The "official" suggestion is to [have separated catch clauses](http://findbugs.sourceforge.net/bugDescriptions.html#REC_CATCH_EXCEPTION)
   
   We didn't notice it on master because during the Maven restore we accidentaly lost the spotbugs execution but it's still on branch-4.14
   
   ### Changes
   * Fix spotbugs
   * Restore spotbugs check w/ jdk8 on CI 
   
   We'll need to cherry-pick to branch-4.14 to fix the compilation on jdk8


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] nicoloboschi commented on pull request #3160: [build] Fix various spotbugs warnings

Posted by GitBox <gi...@apache.org>.
nicoloboschi commented on pull request #3160:
URL: https://github.com/apache/bookkeeper/pull/3160#issuecomment-1084676692


   rerun failure checks


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] nicoloboschi commented on a change in pull request #3160: [build] Fix various spotbugs warnings

Posted by GitBox <gi...@apache.org>.
nicoloboschi commented on a change in pull request #3160:
URL: https://github.com/apache/bookkeeper/pull/3160#discussion_r839019455



##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/checksum/DirectMemoryCRC32Digest.java
##########
@@ -83,6 +83,9 @@ public void update(ByteBuf buf) {
             updateBytesMethod = CRC32.class.getDeclaredMethod("updateBytes", int.class, byte[].class, int.class,
                     int.class);
             updateBytesMethod.setAccessible(true);
+        } catch (RuntimeException e) {

Review comment:
       it's okay. it should be apply cleanly on 4.14 branch. All these changes are needed to make spotbugs passes 




-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] dlg99 commented on a change in pull request #3160: [build] Fix spotbugs on JDK8

Posted by GitBox <gi...@apache.org>.
dlg99 commented on a change in pull request #3160:
URL: https://github.com/apache/bookkeeper/pull/3160#discussion_r838793842



##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/checksum/DirectMemoryCRC32Digest.java
##########
@@ -83,6 +83,9 @@ public void update(ByteBuf buf) {
             updateBytesMethod = CRC32.class.getDeclaredMethod("updateBytes", int.class, byte[].class, int.class,
                     int.class);
             updateBytesMethod.setAccessible(true);
+        } catch (RuntimeException e) {

Review comment:
       let's do these changes in a separate PR for master and cherry-pick it into branch-4.14




-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] nicoloboschi commented on a change in pull request #3160: [build] Fix various spotbugs warnings

Posted by GitBox <gi...@apache.org>.
nicoloboschi commented on a change in pull request #3160:
URL: https://github.com/apache/bookkeeper/pull/3160#discussion_r839018691



##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java
##########
@@ -1238,38 +1242,42 @@ public void processResult(int rc, String s, Object ctx) {
      */
     public static boolean format(ServerConfiguration conf,
             boolean isInteractive, boolean force) throws Exception {
-        return runFunctionWithMetadataBookieDriver(conf, driver -> {
-            try {
-                try (RegistrationManager regManager = driver.createRegistrationManager()) {
-                    boolean ledgerRootExists = regManager.prepareFormat();
-
-                    // If old data was there then confirm with admin.
-                    boolean doFormat = true;
-                    if (ledgerRootExists) {
-                        if (!isInteractive) {
-                            // If non interactive and force is set, then delete old data.
-                            doFormat = force;
-                        } else {
-                            // Confirm with the admin.
-                            doFormat = IOUtils
-                                    .confirmPrompt("Ledger root already exists. "
-                                            + "Are you sure to format bookkeeper metadata? "
-                                            + "This may cause data loss.");
+        return runFunctionWithMetadataBookieDriver(conf, new Function<MetadataBookieDriver, Boolean>() {

Review comment:
       just moved to non-lambda to add the suppression




-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] eolivelli merged pull request #3160: [build] Fix various spotbugs warnings

Posted by GitBox <gi...@apache.org>.
eolivelli merged pull request #3160:
URL: https://github.com/apache/bookkeeper/pull/3160


   


-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org