keith-turner commented on a change in pull request #1113:
URL: https://github.com/apache/fluo/pull/1113#discussion_r595586550



##########
File path: 
modules/api/src/main/java/org/apache/fluo/api/client/LoaderExecutor.java
##########
@@ -48,11 +48,12 @@
 
   /**
    * Same as {@link #execute(Loader)} except it returns a future that 
completes upon successful
-   * commit and if an exception is thrown in the loader, it will be relayed 
through the future.
+   * commit and if an exception is thrown in the loader, it will be relayed 
through the future. The
+   * result of the future is the Loader that was successfully executed.
    *
    * @since 2.0.0
    */
-  CompletableFuture<Void> submit(Loader loader);
+  CompletableFuture<Loader> submit(Loader loader);

Review comment:
       @ctubbsii  I tried both of your suggestions locally w/ this branch.   
The first one `<T extends Loader> CompletableFuture<T> submit(T loader);` seems 
to work while the other one did not.  
   
   I used the IT added by this PR to experiment and the following compiles with 
the first suggestion and does not require the casts.
   
   ```java
       CompletableFuture<CounterLoader> future = le.submit(new CounterLoader());
       Assert.assertEquals(future.get().getNewValue().longValue(), 1);
   
       future = le.submit(new CounterLoader());
       Assert.assertEquals(future.get().getNewValue().longValue(), 2);
       
       // ensure passing Loader type itself and not a subtype of Loader works
       Loader someLoader = new CounterLoader();
       CompletableFuture<Loader> future2 = le.submit(someLoader);
       
Assert.assertEquals(((CounterLoader)future2.get()).getNewValue().longValue(), 
3);
   ```
   
   When trying the 2nd suggestion w/ the above code, I see errors like the 
following.
   
   ```
   Type mismatch: cannot convert from CompletableFuture<capture#2-of ? super 
LoaderExecutorIT.CounterLoader> to 
CompletableFuture<LoaderExecutorIT.CounterLoader>       LoaderExecutorIT.java   
/fluo-integration-tests/src/main/java/org/apache/fluo/integration/client        
line 149        Java Problem
   ```
   




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


Reply via email to