Asynchronous request support

[dwsupplee]

Using this issue as a place to house research and share ideas as to how we can implement async request support.

[bshaffer]

Async has been very difficult to implement in pubsub. I was able to implement it with the following steps:

1. Creating a Connection subclass to override the httpHandler, change the return value of pull, and add the CurlHandlerMulti handler to Guzzle.
2. Setting the pubsub $connection public property to my Connection subclass. I am not sure this is intentionally public.
3. Add a bogus for loop so that the pull function would execute, because it's a generator.

This is not an easy issue to solve, and my solution involves quite a bit of hack. I like Guzzle's approach, where under the hood everything handles being asynchronous, and the sync call use the async functions and call wait() after. But this would require an entire rewrite.

Another (possibly ill-advised) solution would be to have a promise subclass which, upon completion, created a dummy ConnectionInterface implementation to return the value, and passes this ConnectionInterface to the wrapper objects, thereby applying the translations w/o code duplication.

It would be also be sufficient if async calls just returned a Guzzle Promise, though 😄

[jdpedrie]

We've been talking about your last suggestion regarding returning a Promise as our likely path. The Public API would expand quite a bit, essentially offering an async version of each method. For instance, Topic::create() would be paired with Topic::createAsync(). Under the hood, the synchronous methods would use the async method, and resolve the promise before returning a result. The async method would, as you suggest, return a promise.

// php 7 return types for demonstration

public function create($options = []) : array
{
    $promise = $this->createAsync($options);
    $result = null;
    $promise->resolve(function($res) use ($result) {
         $result = $res;
    });
    $promise->wait();

    return $result;
}

public function createAsync($options = []) : Promise
{
    return $this->connection->create();
}

This code is off the top of my head, so forgive errors!

We don't want variability in our return types, otherwise I'd probably like the idea of subclassing everything.

[bshaffer]

I'm not a huge fan of adding an async method for every function, but I can't think of a better way to do this, other than using the __call magic method.

[dwsupplee]

Put together a few thoughts on this :)

__call vs hardcoding synchronous and asynchronous methods

If our synchronous methods are simply going to wrap the async calls with a ->wait() call, I'm a big fan of using __call to reduce the boilerplate we would need to maintain.

I could picture something roughly like this and documenting the methods like so, what do you guys think?

Doc generation

If we use __call we'll need to figure out a method to make sure we can still generate docs with things like example snippets.

gRPC Support

gRPC currently does not support asynchronous calls.

• What should happen when a user opts for a gRPC connection and calls an asynchronous method?
  • @jdpedrie and I spoke briefly about the idea of just allowing the calls to the asynchronous methods provide a notice to the user to let them know the requests are actually being executed synchronously. This way we can allow code to execute normally so nothing needs to change when asynchronous support is eventually included.
  • Throw an ugly exception when connection == grpc && method == async. Not a huge fan of this idea!
• Eventually we expect there to be two types of ways to interact with the cloud services. There will be veneers such as what we have currently with storage/bigquery and there will be toolkit generated code for services which may not be complex enough to warrant a veneer. We should make sure both types of classes share a similiar structure so that users aren't confused as to why *-Async exists in the veneer code but not elsewhere.

Documenting return types

This is an example of something we will need to handle. Users will need to know what they're going to be getting as a value from the promise. As far as I know there isn't a standard for how this should be handled.

• @jdpedrie had an interesting suggestion with borrowing the generics syntax. @returns Promise <Bucket>
• What about just a description? @returns Promise A promise which is fulfilled with a Bucket object. The downside I see here is it can become trickier to parse out the value if that is ever something we may need to do.

Handling methods which can be paginated

@bshaffer's pull example is a good example of how this will be problematic. Getting a generator or iterator from a promise doesn't really make sense. To help solve this my suggestion is to rename buckets to bucketsWithAutoPagination which will act as it does today and continue to return a generator. This will be useful for users who don't want to think about what's going on under the hood. buckets/bucketsAsync will instead need to be paginated manually (assuming they have enough items to iterate over that they aren't returned in a single request) by passing in the next page's token on each request. Check out a rough idea here.

[bshaffer]

Just adding to this, the Logging API will not be useful for general logging w/o async support.

As to the "rough idea", I think this is a great way to go, although I would prefer to avoid using __call and explicitly define each method. I like how, in your example, connection->call returns promises across the board. This makes it easier to make async calls not explicitly defined in the wrapper classes.

[michaelbausor]

Issue for toolkit/GAPIC async support: googleapis/gapic-generator#1408

[michaelbausor]

I have added this to the feature request wiki page, so closing the issue