GoogleCloudPlatform / grpc-gcp-php

Apache License 2.0
165 stars 17 forks source link

Swap from wrapped Client to pure gRPC Client #4

Open ZhouyihaiDing opened 6 years ago

ZhouyihaiDing commented 6 years ago

The FirestoreClient you use is download from Google Cloud by composer, which actually is wrapper around gRPC. This API is designed to face the customers, with some gRPC parameters unseen by the user. However, we want to test the gRPC features with this tests, which means the Client should be pure gRPC Client generated directly from proto files. The only thing needed to be downloaded by composer is auth API..

There are 2 ways to changing your current wrapper Client to pure gRPC Client, which I recommend 2:

  1. use the FirestoreClient under vendor/google/proto-client/src/Google/Cloud/Firestore/V1beta1/FirestoreGrpcClient.php

  2. Generate FirestoreClient by your own. For example, my way to generate pb files:

    git clone git@github.com:googleapis/googleapis.git
    cd googleapis
    make LANGUAGE=php OUTPUT=~/grpc-gcp-php/firestore/examples/end2end/src

Then change your current Client to the one in ~/grpc-gcp-php/firestore/examples/end2end/src/Google/Cloud/Firestore/V1beta1/FirestoreClient.php, and then set the credentials with it. I also have a related example of creating pure Logging gRPC client for you as reference.

burkl commented 6 years ago

unfortunately I don't have any permissions anymore to access the repo.

Could you please grant access to me and my team-mate Slavey (@slaff)? Thanks.

Zhouyihai Ding notifications@github.com schrieb am Di., 30. Jan. 2018 um 02:24 Uhr:

Assigned #4 https://github.com/GoogleCloudPlatform/grpc-gcp-php/issues/4 to @burkl https://github.com/burkl.

— You are receiving this because you were assigned.

Reply to this email directly, view it on GitHub https://github.com/GoogleCloudPlatform/grpc-gcp-php/issues/4#event-1447607928, or mute the thread https://github.com/notifications/unsubscribe-auth/AALCVp6cenWQ8LjNngu13lHdI9Mo292pks5tPm9CgaJpZM4RxniJ .

ZhouyihaiDing commented 6 years ago

Granted. It's strange because I can see you are in the "collaborators" of this repo.

ZhouyihaiDing commented 6 years ago

I'll try remove you from the "collaborators" and re-send the invitation again.

burkl commented 6 years ago

I've opened a pull request which allows you to merge with the composer files.

In the example code we're relying on ./vendor/google/cloud-firestore/V1beta1/FirestoreClient.php which is generated from a proto file and has the same content as the file you've mentioned in 1.

Why do you recommend to generate the files on your own? I expect to get the same content as it is distributed with packagist/composer?

ZhouyihaiDing commented 6 years ago

It's not exact the same. class FirestoreClient extends FirestoreGapicClient and FirestoreGapicClient is wrapper around pure gRPC. If we want to test features like cancel, flow control, we can't set it with FirestoreGapicClient. Also, this wrapper is designed by other team, and if they change their code, we may have to change our test too. class FirestoreGrpcClient extends \Grpc\BaseStub. It creates a client directly from \Grpc\BaseStub. Thus we can do what we want when testing gRPC.

ZhouyihaiDing commented 6 years ago

I am not sure where 1 comes from. Is that already generated or dynamically generated. If we update our generator plugin, will 1 update it's generated code at the same time? This is the reason I recommend 2.

slaff commented 6 years ago

@ZhouyihaiDing I tried to follow your instructions but I am facing multiple issues. So let me briefly explain what I did and where are the issues:

  1. I get the latest version of protoc for Linux
    
    wget https://github.com/google/protobuf/releases/download/v3.5.1/protoc-3.5.1-linux-x86_64.zip -O protoc.zip

Unzip

unzip protoc.zip -d protoc3

Move protoc to /usr/local/bin/

sudo mv protoc3/bin/* /usr/local/bin/

Move protoc3/include to /usr/local/include/

sudo mv protoc3/include/* /usr/local/include/


And then tried to compile the api, as recommended.

Git Clone The API

git clone https://github.com/googleapis/googleapis.git

cd googleapis make LANGUAGE=php OUTPUT=../output-php


During the compilation I was faced with the following issues:

1. `Can only generate PHP code for # proto3 .proto files. Please add 'syntax = "proto3";' to the top of your .proto file.`. I had to edit `/usr/local/include/google/protobuf/ compiler/plugin.proto` file.
2. All proto files that have version 3 throw errors if there is an optional keyword. Example: 

protoc --proto_path=.:/usr/local/include --php_out=../output-php --grpc_out=../output-php --plugin=protoc-gen-grpc=/usr/local/bin/grpc_php_plugin google/monitoring/v3/metric_service.proto google/protobuf/descriptor.proto:601:12: Explicit 'optional' labels are disallowed in the Proto3 syntax. To define 'optional' fields in Proto3, simply remove the 'optional' label, as fields are 'optional' by default.

I had to edit some files but then was faced with the final issue, that I do not know how to solve:

protoc --proto_path=.:/usr/local/include --php_out=../output-php --grpc_out=../output-php --plugin=protoc-gen-grpc=/usr/local/bin/grpc_php_plugin google/monitoring/v3/metric_service.proto

google/protobuf/descriptor.proto: Extension ranges are not allowed in proto3. google/protobuf/descriptor.proto: Explicit default values are not allowed in proto3. google/protobuf/descriptor.proto: Required fields are not allowed in proto3.

followed by

google/api/annotations.proto: Import "google/protobuf/descriptor.proto" was not found or had errors.



So my question is what I am doing wrong? Is there a version of `protoc` that supports the older v2 syntax? 
ZhouyihaiDing commented 6 years ago

Hi @slaff , I think it's caused by missing /usr/local/bin/grpc_php_plugin. I am sorry currently there is no easy way to install grpc_php_plugin. The only way is to compile grpc from source. Below is my step by step notes I recorded to install pre-request on an empty ubuntu VM. You can add them into README if you want: # install packages

sudo apt-get install build-essential autoconf libtool
sudo apt-get install zip unzip zlib1g-dev
sudo apt-get install php php-dev
sudo pecl install grpc
curl -sS https://getcomposer.org/installer | php
sudo mv composer.phar /usr/local/bin/composer

# Install protoc from protobuf:

cd ~
git clone git@github.com:google/protobuf.git
cd protobuf
./autogen.sh
./configure
make -j8
sudo make install
export LD_LIBRARY_PATH=/usr/local/lib

# install grpc_[language]_plugin from grpc:

git clone git@github.com:grpc/grpc.git
cd grpc
git submodule update --init
make -j8    
# or make grpc_php_plugin
sudo make install

# Generate pb files:

cd ~
git clone https://github.com/googleapis/googleapis.git
...
make LANGUAGE=php OUTPUT=~/grpc-gcp-php/firestore/examples/end2end/src
slaff commented 6 years ago

Implemented in PR #6