Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Swap from wrapped Client to pure gRPC Client #4

Open
ZhouyihaiDing opened this issue Jan 30, 2018 · 9 comments
Open

Swap from wrapped Client to pure gRPC Client #4

ZhouyihaiDing opened this issue Jan 30, 2018 · 9 comments
Assignees

Comments

@ZhouyihaiDing
Copy link
Contributor

ZhouyihaiDing commented Jan 30, 2018

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 [email protected]: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
Copy link
Contributor

burkl commented Jan 31, 2018 via email

@ZhouyihaiDing
Copy link
Contributor Author

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

@ZhouyihaiDing
Copy link
Contributor Author

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

@burkl
Copy link
Contributor

burkl commented Feb 1, 2018

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
Copy link
Contributor Author

ZhouyihaiDing commented Feb 1, 2018

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
Copy link
Contributor Author

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

slaff commented Feb 8, 2018

@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
Copy link
Contributor Author

ZhouyihaiDing commented Feb 8, 2018

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 [email protected]: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 [email protected]: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
Copy link

slaff commented Feb 15, 2018

Implemented in PR #6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants