westonpace commented on PR #37514:
URL: https://github.com/apache/arrow/pull/37514#issuecomment-1710489723

   I'll try and look in more detail later today but here is the general advice.
   
   Async methods which return a future.
   Sync methods do not
   
   User calls always start out as synchronous methods (e.g. we don't have 
adapters for python's asyncio)
   
   We can convert from synchronous to asynchronous using something like...
   
   ```
   ... Foo(...) {
       Future<...> fut = FooAsync(...);
       return fut.result();
   }
   ```
   
   This is ok.  One conversion from synchronous to asynchronous is ok and 
inevitable (since user calls start as sync).
   
   However, it is not ok to convert from asynchronous to synchronous.  For 
example, if `Foo` is defined as above then this would not be ok:
   
   ```
   ... BarAsync(...) {
      Future<...> fut = SomeAsyncMethod();
      return fut.Then([&] (...) {
          return Foo(...);
      });
   }
   ```
   
   This is also not ok:
   
   ```
   ... BarAsync(...) {
       auto x = Foo(...);
       return SomeAsyncMethod(x, ...);
   }
   ```
   
   Normally the fix is for `BarAsync` to call `FooAsync`.
   
   Creating a `Bar` that is a synchronous method that mirrors `BarAsync` is ok, 
but leads to more code.
   
   At a glance, it looks like that is what is happening here, which is maybe 
ok.  However, I have to look in more detail.


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