ndimiduk commented on code in PR #6798:
URL: https://github.com/apache/hbase/pull/6798#discussion_r1997511411


##########
hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java:
##########
@@ -758,16 +758,13 @@ public CompletableFuture<Void> disableTable(TableName 
tableName) {
    * targetState).
    */
   private static CompletableFuture<Boolean> completeCheckTableState(
-    CompletableFuture<Boolean> future, TableState tableState, Throwable error,
+    CompletableFuture<Boolean> future, Optional<TableState> tableState, 
Throwable error,

Review Comment:
   I don't think that this change solves the source of the NPE. Per the stack 
trace, `tableState` was `null` entering the lambda on line 797. I was expecting 
to find a null-check or use of `Optional.ofNullable` in order to ride over the 
`null` value.
   
   I think that to solve the stack trace, you need to guard against a `null` 
value being provided here in `completeCheckTableState` for the `tableState` 
parameter. You could use a basic `if (tableState == null)`, or an `Optional` 
chain that unpacks the value, something like:
   
   ```
   
Optional.ofNullable(tableState).flatMap(Function.identity).ifPresentOrElse(...)
   ```
   
   I agree that passing the `Optional` all the way down is a nicer approach.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to