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
