merge from pubsub-grpc: Initial version of Toolkit\PubSub\V1

[garrettjonesgoogle]

No description provided.

[dwsupplee]

A few more items to discuss:

1. Fresh tokens are being negotiated for every call in the same script. We may want to consider abstracting away any handling of tokens from GAX/gRPC or provide some type of caching.
2. Using the objects generated by https://github.com/drslump/Protobuf-PHP as params and return values could be problematic. This can lead to complex configuration that isn't well documented anywhere. It is especially difficult as these classes are dependencies living in other repos.

$formattedName = SubscriberApi::formatSubscriptionName('project', 'subscription');
$formattedTopic = SubscriberApi::formatTopicName('project', 'topic');

$subscriber->createSubscription($formattedName, $formattedTopic, [
    'pushConfig' => new PushConfig() // For a user I can see this being *very* confusing. What is this? How do I configure it?
]); 

Also if the plan is to replace the drslump library with a proprietary google one it could be possible to run into BC breaks. It would be nice to have these kind of issues abstracted away.

3 . Objects are not deserializing properly as there is an issue with gRPC. The repositories keyword applies to the root context only, meaning the forked code with the deserialize method (and possibly other changes) won't make it in to gcloud-php or whatever other projects include the gRPC dependency.

4 . We are using auth 0.9 while gRPC and GAX are tied to 0.7. If it is at all possible to remove the auth dependency from gRPC and GAX that would allow us much greater flexibility.

5 . We should exclude the src/Toolkit directory from our test suite

[garrettjonesgoogle]

@dwsupplee : which of those items do you feel need to be fixed before merging to master, and how many can be filed as issues to fix later? Or do we need a discussion before we can decide?

[dwsupplee]

@garrettjonesgoogle A discussion would be really helpful. Could we try and schedule something for monday with @jdpedrie?

[dwsupplee]

@garrettjonesgoogle

This is how you can exclude the generated code from the code coverage report:

https://github.com/GoogleCloudPlatform/gcloud-php/blob/master/phpunit.xml.dist

<?xml version="1.0" encoding="UTF-8"?>
<phpunit bootstrap="./tests/bootstrap.php" colors="true">
  <testsuites>
    <testsuite>
      <directory>tests</directory>
    </testsuite>
  </testsuites>
  <filter>
    <whitelist>
      <directory suffix=".php">src</directory>
      <exclude>
        <directory suffix=".php">src/PubSub/V1</directory>
      </exclude>
    </whitelist>
  </filter>
  <logging>
    <log type="coverage-clover" target="build/clover.xml"/>
  </logging>
</phpunit>

[dwsupplee]

Hey @garrettjonesgoogle!

What are your thoughts on moving proto-client-php right here alongside the toolkit generated code? I bring this up because the version of gRPC being relied on in the library is out of date and moving it would mean one less dependency to have to worry about.

Also, are there any updates on the namespace being modified?

Thanks! :)

[garrettjonesgoogle]

Hey @dwsupplee , I think we need proto-client-php to remain separate for when we expand the scope of the toolkit to generate client libraries for non-cloud APIs.

I was waiting to do any modifications to the code until more stuff is unblocked (e.g. proto3 is supported). If the namespace rename is blocking you, we can get that done.

[dwsupplee]

That seems fair! I wasn't sure because the readme says it was for the google cloud platform.

The namespace isn't an issue currently, it will be pretty quick to update once the change is ready for us.

[dwsupplee]

Hey, @garrettjonesgoogle :).

I just realized the Publisher and Subscriber APIs don't match 100% with what we have for the REST implementation. It doesn't look like there is a way to manage IAM policies. Is this something we could get generated as well?

[garrettjonesgoogle]

Hey @dwsupplee , the missing IAM messages are actually a gap across languages. We have a pending team work item to design the solution for that.

[dwsupplee]

@jgeewax do you have any thoughts on how we can move forward handling IAM?

[michaelbausor]

Additions in these latest commits:

• Added surface for Logging API
• Support for passing page tokens into page streaming methods
• Merged optionalArgs and callSettings
• Updated namespace
• Added credentialsLoader optional argument to XApi classes

[michaelbausor]

Additions in these commits:

• Change @var to @type
• Exclude pubsub and logging from tests
• Incorporate latest gax-php changes

[garrettjonesgoogle]

IAM methods added! PTAL.

[garrettjonesgoogle]

Is anything blocking this from being merged?

[dwsupplee]

I have set aside some time tomorrow morning to review the new IAM methods. We will get this merged in right afterwards. :)

[dwsupplee]

LGTM. Thanks!