[GitHub] [bookkeeper] merlimat commented on a change in pull request #2723: Refactor book keeper cluster test case

[GitBox <gi...@apache.org>]

merlimat commented on a change in pull request #2723:
URL: https://github.com/apache/bookkeeper/pull/2723#discussion_r639983590



##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java
##########
@@ -504,6 +504,12 @@ public boolean isSecure() {
         };
     }
 
+    public static String buildStatsLoggerScopeName(BookieId addr) {

Review comment:
       The name of this method can be misleading. We already removed the bookie address from the metric names (instead it's added as a label). Sine this appears to be only by tests, we should just move the methods to tests folder.

##########
File path: bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieWriteToJournalTest.java
##########
@@ -80,7 +79,9 @@ void readJournal() throws IOException, BookieException {
     /**
      * test that Bookie calls correctly Journal.logAddEntry about "ackBeforeSync" parameter.
      */
+
     @Test
+//    @Ignore("PLSR-1850 test is failing due to Bookie interface change")

Review comment:
       ```suggestion
   ```




-- 
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