Added grpc extension. - #62

Is @stanley-cheung interested in submitting his changes datto/protobuf-php? Why are we using his fork?

We tried, but the upstream repo has not responded in a year: drslump/Protobuf-PHP#48

But because he has submited his package to Packagist, we can't submit our fork unless we change the repo name.

That was why I have to resort to this composer.json syntax to use his official package on Packagist in the require section but substitute the actual content with our fork.

Shouldn't this get pulled in implicitly as part of the composer requirements for grpc?

That is true. By pulling grpc/grpc, you are also implicitly pulling datto/protobuf-php (with a substituted fork) yes.

Edit: sorry, I take that back. The composer.json file you should be looking at should be at the top level of the grpc repo: https://github.com/grpc/grpc/blob/master/composer.json. Packagist only look at composer.json at the top level of a repo. There we did not pull in datto/protobuf-php. I believe at that time we thought pulling in datto/protobuf-php should be left to user of grpc.

Ok, so there's no need for specifying datto/protofuf-php here. I will remove it

No problem, I added it back.

OK gotcha. I advocate making it the default, since it will be very difficult for users to discover the need for them to override with your fork on their own.

So I just tried this again yesterday. Even if the root level grpc/composer.json is going to pull in datto/protobuf-php and with my stanley-cheung override, as a user of grpc/grpc, you are not going to get the stanley-cheung override unless the user also puts that in their own composer.json. In other words, as a user of grpc/grpc, at the minimum, they still need to require grpc/grpc and add the stanley-cheung/protobuf-php override. But then at least they don't need to include datto/protobuf-php. It's not ideal but I can spell it out more in grpc's documentation.

@stanley-cheung Yes, we should add this to composer. Also, are you sure the override is required in the userland composer.json? If so, that seems like a bug in composer. And also if so, we could submit your fork as a new composer package, which would definitely fix the issue.

Requiring the users to override in composer.json rather than just call composer require grpc/grpc is not acceptable DX IMHO. We could fork to GoogleCloudPlatform and submit a new package from there.

Yes I have tried this again and an end-user needs to do this override, and I agree that it's non-ideal. OTOH, this was supposed to be a temporary bridgegap before we have official PHP support in google/protobuf - it's being implemented now. So if we can wait for a few months, this will all go away and we will have the official solution.

Hey who are you man!

It would be nice if we could use the credentials' getUpdateMetadataFunc function for this:

$auth_credentials = ApplicationDefaultCredentials::getCredentials(
     ['https://www.googleapis.com/auth/pubsub']
);
$call = $client->ListTopics(
     $req,
     [],
     ['call_credentials_callback' => $auth_credentials->getUpdateMetadataFunc())];

But it looks like since the first argument doesn't match up, this won't work. Maybe we should change the google/auth library to handle this? I assume that is what this function is meant for.

hm, so this callback 'updateAuthMetadataCallback' is meant to be seen in gRPC's perspective. It's as if the gRPC core library is saying, "I am ready for the auth credentials. Here's what we know about this call so far, contained in the $context variable (the most important one being $context->service_url, coz after load balancing, user may not know which is the host until now)". So with this $context variable, we ask the auth library for the auth header by calling $auth_credentials->updateMetadata passing $context->service_url to it.

To me these 2 callbacks don't need to lineup because the outer gRPC callback can do more than just Auth. I think it;s OK to leave this as is.

Ok, thanks for the explanation. Is it a problem that the google/auth library isn't doing anything with the service_url?

Also, we could make this cleaner like this:

$auth_credentials = ApplicationDefaultCredentials::getCredentials(
     ['https://www.googleapis.com/auth/pubsub']
);
$call = $client->ListTopics(
     $req,
     [],
     ['call_credentials_callback' => function($context) use ($auth_credentials) {
         return $auth_credentials->updateMetadata([], $context->service_url);
     })];

I believe the $service_url is used only in some xxxCredentials class, like ServiceAccountJwtAccessCredentials but not the others.

I like the inline function change.

ahh yes, I see that now. Very cool. SGTM, thanks @stanley-cheung!

Done