proto: enable compiling gRPC services #2800
Conversation
Summary:
This commit teaches `tb_proto_library` a new `has_services` flag. When
set, the proto library will be compiled with gRPC service support. The
flag name and implementation are adapted from TensorFlow’s.
Indentation changes are due to buildifier.
Test Plan:
Append a simple service to `tensorboard/plugins/hparams/api.proto`:
```proto
service TestService {
rpc ListSesssionGroups(ListSessionGroupsRequest)
returns (ListSessionGroupsResponse);
}
```
Build the Pip package and install it into a new virtualenv. Try to
import `tensorboard.compat.proto.test_service_pb2_grpc`, but note that
this fails. Then, add `has_services = True` to the hparams `BUILD` file,
and rebuild and reinstall the Pip package: note that the import now
works, and the module has a `TestServiceStub` attribute.
wchargin-branch: proto-grpc
|
Can you confirm that this change works internally as well? It seems to me that the external py_proto_library knows about use_grpc_plugin, but I'm not clear that the internal one does. (Note TF redefines the py_proto_library rule, so copying that usage may not work as expected). If this works as is, great! But if not, maybe there is also a copybara thing needed too, etc. (?) |
|
Oh nevermind, protos.bzl is only for external use anyway. LGTM |
|
Right; this will have an accompanying internal change. |
|
I will restructure this to more cleanly separate the |
wchargin-branch: proto-grpc wchargin-source: 363716e75b3ce34a720c014d2c9a726afce54cf6
Thanks for this prompt—you’re correct that this |
|
Also cc @caisq, who has some experience with gRPC in open source. If |
davidsoergel
left a comment
davidsoergel
left a comment
There was a problem hiding this comment.
Seems reasonable. Also Cc @jameswex; IIRC Facets did the same thing (committing generated protos) so maybe this approach would apply there as well.
|
@wchargin The debugger plugin checks in generated Python code because at the time that was done (mid 2017), there was no py grpc genrule that worked both internally and externally yet. |
|
Great; thanks, all. |
Summary:
This commit teaches `tb_proto_library` a new `has_services` flag. When
set, the proto library will be compiled with gRPC service support, and
an additional `*_py_pb2_grpc` `py_library` target will be created.
The `py_proto_library` macro provided by the protobuf build rules no
longer suits our purpose, as it can only create one `*_py_pb2` build
target that includes both the `*_pb2` and `*_pb2_grpc` Python modules.
This is inconsistent with TensorFlow and some Google internal rules. We
now directly use `proto_gen` rather than using `py_proto_library`. This
is fine: that macro is marked as unstable because its interface may
change, and our goal is precisely to change the interface.
Test Plan:
Run
```
bazel query --output=build \
'filter(".*protos_all.*", //tensorboard/...:all)'
```
and note that the output is identical before and after this change, so
existing protos will be unchanged.
Then, add a simple service to `tensorboard/plugins/hparams/api.proto`:
```proto
service TestService {
rpc ListSesssionGroups(ListSessionGroupsRequest)
returns (ListSessionGroupsResponse);
}
```
Build the Pip package and install it into a new virtualenv. Try to
import `tensorboard.plugins.hparams.api_pb2_grpc`, but note that
this fails. Then, add `has_services = True` to the hparams `BUILD` file,
and rebuild and reinstall the Pip package. The import should still fail.
Finally, add a `:protos_all_py_pb2_grpc` dep to `:hparams_plugin`, and
try once more. The import should now work, and the imported module
should have a `TestServiceStub` attribute.
wchargin-branch: proto-grpc
Summary:
This commit teaches `tb_proto_library` a new `has_services` flag. When
set, the proto library will be compiled with gRPC service support, and
an additional `*_py_pb2_grpc` `py_library` target will be created.
The `py_proto_library` macro provided by the protobuf build rules no
longer suits our purpose, as it can only create one `*_py_pb2` build
target that includes both the `*_pb2` and `*_pb2_grpc` Python modules.
This is inconsistent with TensorFlow and some Google internal rules. We
now directly use `proto_gen` rather than using `py_proto_library`. This
is fine: that macro is marked as unstable because its interface may
change, and our goal is precisely to change the interface.
Test Plan:
Run
```
bazel query --output=build \
'filter(".*protos_all.*", //tensorboard/...:all)'
```
and note that the output is identical before and after this change, so
existing protos will be unchanged.
Then, add a simple service to `tensorboard/plugins/hparams/api.proto`:
```proto
service TestService {
rpc ListSesssionGroups(ListSessionGroupsRequest)
returns (ListSessionGroupsResponse);
}
```
Build the Pip package and install it into a new virtualenv. Try to
import `tensorboard.plugins.hparams.api_pb2_grpc`, but note that
this fails. Then, add `has_services = True` to the hparams `BUILD` file,
and rebuild and reinstall the Pip package. The import should still fail.
Finally, add a `:protos_all_py_pb2_grpc` dep to `:hparams_plugin`, and
try once more. The import should now work, and the imported module
should have a `TestServiceStub` attribute.
wchargin-branch: proto-grpc
4 participants
Summary:
This commit teaches
tb_proto_librarya newhas_servicesflag. Whenset, the proto library will be compiled with gRPC service support, and
an additional
*_py_pb2_grpcpy_librarytarget will be created.The
py_proto_librarymacro provided by the protobuf build rules nolonger suits our purpose, as it can only create one
*_py_pb2buildtarget that includes both the
*_pb2and*_pb2_grpcPython modules.This is inconsistent with TensorFlow and some Google internal rules. We
now directly use
proto_genrather than usingpy_proto_library. Thisis fine: that macro is marked as unstable because its interface may
change, and our goal is precisely to change the interface.
Test Plan:
Run
and note that the output is identical before and after this change, so
existing protos will be unchanged.
Then, add a simple service to
tensorboard/plugins/hparams/api.proto:Build the Pip package and install it into a new virtualenv. Try to
import
tensorboard.plugins.hparams.api_pb2_grpc, but note thatthis fails. Then, add
has_services = Trueto the hparamsBUILDfile,and rebuild and reinstall the Pip package. The import should still fail.
Finally, add a
:protos_all_py_pb2_grpcdep to:hparams_plugin, andtry once more. The import should now work, and the imported module
should have a
TestServiceStubattribute.wchargin-branch: proto-grpc