steveloughran commented on code in PR #4039: URL: https://github.com/apache/hadoop/pull/4039#discussion_r1138820601
########## hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java: ########## @@ -702,7 +707,7 @@ public AbfsRestOperation append(final String path, final byte[] buffer, */ int responseStatusCode = ((AbfsRestOperationException) e).getStatusCode(); if (checkUserError(responseStatusCode) && reqParams.isExpectHeaderEnabled()) { - LOG.debug("User error, retrying without 100 continue enabled"); + LOG.debug("User error, retrying without 100 continue enabled for the given path " + path); Review Comment: nit; use {} in the string, path as an argument * skips string concat when logging doesn't take place * if path is null, triggers error on toString(), logging recovers ########## hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClientThrottlingIntercept.java: ########## @@ -160,7 +160,7 @@ public void updateMetrics(AbfsRestOperationType operationType, throttling but there were some expectedBytesToBeSent. */ if (updateBytesTransferred(isThrottledOperation, abfsHttpOperation)) { - LOG.debug("Updating metrics due to throttling"); + LOG.debug("Updating metrics due to throttling for path " + abfsHttpOperation.getConnUrl().getPath()); Review Comment: same thing about {} ########## hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsClient.java: ########## @@ -579,8 +577,6 @@ public void testExpectHundredContinue() throws Exception { .isEqualTo(0); // Verify that the same request was retried with expect header disabled. - Assertions.assertThat(appendRequestParameters.isExpectHeaderEnabled()) - .describedAs("The expect header is not false") - .isEqualTo(false); + assertFalse(appendRequestParameters.isExpectHeaderEnabled()); Review Comment: sorry, i must have explained it wrong. AssertJ has assert/true false too in its type `AbstractBooleanAssert`. you can go ``` Assertions.assertThat(appendRequestParameters.isExpectHeaderEnabled()) .describedAs("The expect header is not false") .isFalse() ``` so can you restore what was cut and just change the assert. ``` -- 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: common-issues-unsubscr...@hadoop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org