From f13b3249414240ce0a28110ba1a7f6732a6c3ec8 Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Mon, 19 Oct 2020 13:02:33 -0700 Subject: [PATCH 1/6] Include well-known types in sys.path when using runtime protos --- .../grpcio_tests/tests/unit/BUILD.bazel | 1 + .../tests/unit/_dynamic_stubs_test.py | 47 ++++++++++++++----- .../tests/unit/data/foo/baz.proto | 27 +++++++++++ tools/distrib/python/grpcio_tools/BUILD.bazel | 1 + .../python/grpcio_tools/grpc_tools/protoc.py | 2 + 5 files changed, 66 insertions(+), 12 deletions(-) create mode 100644 src/python/grpcio_tests/tests/unit/data/foo/baz.proto diff --git a/src/python/grpcio_tests/tests/unit/BUILD.bazel b/src/python/grpcio_tests/tests/unit/BUILD.bazel index 909ced21383..64dcab29615 100644 --- a/src/python/grpcio_tests/tests/unit/BUILD.bazel +++ b/src/python/grpcio_tests/tests/unit/BUILD.bazel @@ -125,6 +125,7 @@ py2and3_test( srcs = ["_dynamic_stubs_test.py"], data = [ "data/foo/bar.proto", + "data/foo/baz.proto", ], imports = ["../../"], main = "_dynamic_stubs_test.py", diff --git a/src/python/grpcio_tests/tests/unit/_dynamic_stubs_test.py b/src/python/grpcio_tests/tests/unit/_dynamic_stubs_test.py index 6a4387c547c..ed107742b67 100644 --- a/src/python/grpcio_tests/tests/unit/_dynamic_stubs_test.py +++ b/src/python/grpcio_tests/tests/unit/_dynamic_stubs_test.py @@ -21,6 +21,8 @@ import os import sys import unittest +_DATA_DIR = os.path.join("tests", "unit", "data") + @contextlib.contextmanager def _grpc_tools_unimportable(): @@ -53,6 +55,18 @@ def _collect_errors(fn): return _wrapped +def _python3_check(fn): + + @functools.wraps(fn) + def _wrapped(): + if sys.version_info[0] == 3: + fn() + else: + _assert_unimplemented("Python 3") + + return _wrapped + + def _run_in_subprocess(test_case): sys.path.insert( 0, os.path.join(os.path.realpath(os.path.dirname(__file__)), "..")) @@ -80,24 +94,30 @@ def _assert_unimplemented(msg_substr): @_collect_errors +@_python3_check def _test_sunny_day(): - if sys.version_info[0] == 3: - import grpc - protos, services = grpc.protos_and_services( - os.path.join("tests", "unit", "data", "foo", "bar.proto")) - assert protos.BarMessage is not None - assert services.BarStub is not None - else: - _assert_unimplemented("Python 3") + import grpc + protos, services = grpc.protos_and_services( + os.path.join(_DATA_DIR, "foo", "bar.proto")) + assert protos.BarMessage is not None + assert services.BarStub is not None @_collect_errors +@_python3_check +def _test_well_known_types(): + import grpc + protos, services = grpc.protos_and_services( + os.path.join(_DATA_DIR, "foo", "baz.proto")) + assert protos.BarMessage is not None + assert services.BarStub is not None + + +@_collect_errors +@_python3_check def _test_grpc_tools_unimportable(): with _grpc_tools_unimportable(): - if sys.version_info[0] == 3: - _assert_unimplemented("grpcio-tools") - else: - _assert_unimplemented("Python 3") + _assert_unimplemented("grpcio-tools") # NOTE(rbellevi): multiprocessing.Process fails to pickle function objects @@ -109,6 +129,9 @@ class DynamicStubTest(unittest.TestCase): def test_sunny_day(self): _run_in_subprocess(_test_sunny_day) + def test_well_known_types(self): + _run_in_subprocess(_test_well_known_types) + def test_grpc_tools_unimportable(self): _run_in_subprocess(_test_grpc_tools_unimportable) diff --git a/src/python/grpcio_tests/tests/unit/data/foo/baz.proto b/src/python/grpcio_tests/tests/unit/data/foo/baz.proto new file mode 100644 index 00000000000..a2ca7ab524f --- /dev/null +++ b/src/python/grpcio_tests/tests/unit/data/foo/baz.proto @@ -0,0 +1,27 @@ +// Copyright 2020 The gRPC authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +syntax = "proto3"; + +package tests.unit.data.foo.bar; + +import "google/protobuf/wrappers.proto"; + +message BarMessage { + string a = 1; +}; + +service Bar { + rpc GetBar(BarMessage) returns (BarMessage); +}; diff --git a/tools/distrib/python/grpcio_tools/BUILD.bazel b/tools/distrib/python/grpcio_tools/BUILD.bazel index c0b3f66e903..a23975feeb8 100644 --- a/tools/distrib/python/grpcio_tools/BUILD.bazel +++ b/tools/distrib/python/grpcio_tools/BUILD.bazel @@ -50,4 +50,5 @@ py_library( "//src/python/grpcio/grpc:grpcio", "@com_google_protobuf//:protobuf_python", ], + data = glob(["grpc_tools/_proto/**/*"]), ) diff --git a/tools/distrib/python/grpcio_tools/grpc_tools/protoc.py b/tools/distrib/python/grpcio_tools/grpc_tools/protoc.py index d07daba2c06..48f853ad4f2 100644 --- a/tools/distrib/python/grpcio_tools/grpc_tools/protoc.py +++ b/tools/distrib/python/grpcio_tools/grpc_tools/protoc.py @@ -58,6 +58,8 @@ if sys.version_info >= (3, 5, 0): ProtoFinder(_SERVICE_MODULE_SUFFIX, _protoc_compiler.get_services) ]) + sys.path.append( + pkg_resources.resource_filename('grpc_tools', '_proto')) _FINDERS_INSTALLED = True def _module_name_to_proto_file(suffix, module_name): From 3dad0eb862c2a91a2c5143beae6152b72105bc2f Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Mon, 19 Oct 2020 13:07:52 -0700 Subject: [PATCH 2/6] Oops --- examples/python/route_guide/run_codegen.py | 24 ++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 examples/python/route_guide/run_codegen.py diff --git a/examples/python/route_guide/run_codegen.py b/examples/python/route_guide/run_codegen.py new file mode 100644 index 00000000000..8df562d3497 --- /dev/null +++ b/examples/python/route_guide/run_codegen.py @@ -0,0 +1,24 @@ +# Copyright 2015 gRPC authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Runs protoc with the gRPC plugin to generate messages and gRPC stubs.""" + +from grpc_tools import protoc + +protoc.main(( + '', + '-I../../protos', + '--python_out=.', + '--grpc_python_out=.', + '../../protos/route_guide.proto', +)) From 14d6850f5a25bf18012c47a242e43e2a61171d71 Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Mon, 19 Oct 2020 13:09:07 -0700 Subject: [PATCH 3/6] And remove it too --- examples/python/route_guide/run_codegen.sh | 23 ---------------------- 1 file changed, 23 deletions(-) delete mode 100755 examples/python/route_guide/run_codegen.sh diff --git a/examples/python/route_guide/run_codegen.sh b/examples/python/route_guide/run_codegen.sh deleted file mode 100755 index 30666a309d0..00000000000 --- a/examples/python/route_guide/run_codegen.sh +++ /dev/null @@ -1,23 +0,0 @@ -#!/bin/bash -x - -# Copyright 2015 gRPC authors. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -# Runs protoc with the gRPC plugin to generate messages and gRPC stubs - -python3 -m grpc_tools.protoc \ - -I ../../protos \ - --python_out=. \ - --grpc_python_out=. \ - ../../protos/route_guide.proto From 79f54e9d5783a2d521f20a5f5c0e11496709abb3 Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Mon, 19 Oct 2020 23:39:01 -0700 Subject: [PATCH 4/6] Include well known types in bazel build --- tools/distrib/python/grpcio_tools/BUILD.bazel | 10 +- .../python/grpcio_tools/grpcio_tools.bzl | 91 +++++++++++++++++++ 2 files changed, 100 insertions(+), 1 deletion(-) create mode 100644 tools/distrib/python/grpcio_tools/grpcio_tools.bzl diff --git a/tools/distrib/python/grpcio_tools/BUILD.bazel b/tools/distrib/python/grpcio_tools/BUILD.bazel index a23975feeb8..eaf4895b49b 100644 --- a/tools/distrib/python/grpcio_tools/BUILD.bazel +++ b/tools/distrib/python/grpcio_tools/BUILD.bazel @@ -19,6 +19,7 @@ package(default_visibility = [ ]) load("//bazel:cython_library.bzl", "pyx_library") +load("grpcio_tools.bzl", "internal_copied_filegroup") cc_library( name = "protoc_lib", @@ -37,6 +38,13 @@ pyx_library( deps = [":protoc_lib"], ) +internal_copied_filegroup( + name = "well_known_protos", + srcs = ["@com_google_protobuf//:well_known_protos"], + strip_prefix = "src/", + dest = "grpc_tools/_proto/", +) + py_library( name = "grpc_tools", srcs = [ @@ -50,5 +58,5 @@ py_library( "//src/python/grpcio/grpc:grpcio", "@com_google_protobuf//:protobuf_python", ], - data = glob(["grpc_tools/_proto/**/*"]), + data = [":well_known_protos"], ) diff --git a/tools/distrib/python/grpcio_tools/grpcio_tools.bzl b/tools/distrib/python/grpcio_tools/grpcio_tools.bzl new file mode 100644 index 00000000000..8b6c121ffa6 --- /dev/null +++ b/tools/distrib/python/grpcio_tools/grpcio_tools.bzl @@ -0,0 +1,91 @@ +# Copyright 2020 The gRPC authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +def _generate_copied_files_impl(ctx): + srcs = ctx.attr.srcs[0] + strip_prefix = ctx.attr.strip_prefix + dest = ctx.attr.dest + + outs = [] + for f in srcs.files.to_list(): + destination_path = f.path + if f.path.startswith("external"): + external_separator = f.path.find("/") + repository_separator = f.path.find("/", external_separator + 1) + destination_path = f.path[repository_separator+1:] + if not destination_path.startswith(strip_prefix): + fail("File '{}' did not start with '{}'.".format( + destination_path, strip_prefix)) + destination_path = dest + destination_path[len(strip_prefix):] + destination_dir = destination_path.rfind("/") + out_file = ctx.actions.declare_file(destination_path) + outs.append(out_file) + ctx.actions.run_shell( + inputs = [f], + outputs = [out_file], + command = "mkdir -p {0} && cp {1} {2}".format( + out_file.dirname, f.path, out_file.path), + ) + + return [DefaultInfo(files = depset(direct = outs))] + + +_generate_copied_files = rule( + attrs = { + "srcs": attr.label_list( + mandatory = True, + allow_empty = False, + ), + "strip_prefix": attr.string( + default = "", + ), + "dest": attr.string( + mandatory = True, + ) + }, + implementation = _generate_copied_files_impl, +) + + +def internal_copied_filegroup(name, srcs, strip_prefix, dest): + """Copies a file group to the current package. + + Useful for using an existing filegroup as a data dependency. + + Args: + name: The name of the rule. + srcs: A single filegroup. + strip_prefix: An optional string to strip from the beginning + of the path of each file in the filegroup. Must end in a slash. + dest: The directory in which to put the files, relative to the + current package. Must end in a slash. + """ + if len(srcs) != 1: + fail("srcs must be a single filegroup.") + + if not dest.endswith("/"): + fail("dest must end with a '/' character.") + + _symlink_target = name + "_symlink" + _generate_copied_files( + name = _symlink_target, + srcs = srcs, + strip_prefix = strip_prefix, + dest = dest, + ) + + native.filegroup( + name = name, + srcs = [":" + _symlink_target], + ) From b0dfa295a000c70414cd8d680ce2d2c2a7b6226b Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Tue, 20 Oct 2020 09:44:49 -0700 Subject: [PATCH 5/6] Buildifier --- tools/distrib/python/grpcio_tools/BUILD.bazel | 10 +++++----- .../distrib/python/grpcio_tools/grpcio_tools.bzl | 15 +++++++++------ 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/tools/distrib/python/grpcio_tools/BUILD.bazel b/tools/distrib/python/grpcio_tools/BUILD.bazel index eaf4895b49b..e85ea461fb4 100644 --- a/tools/distrib/python/grpcio_tools/BUILD.bazel +++ b/tools/distrib/python/grpcio_tools/BUILD.bazel @@ -39,10 +39,10 @@ pyx_library( ) internal_copied_filegroup( - name = "well_known_protos", - srcs = ["@com_google_protobuf//:well_known_protos"], - strip_prefix = "src/", - dest = "grpc_tools/_proto/", + name = "well_known_protos", + srcs = ["@com_google_protobuf//:well_known_protos"], + dest = "grpc_tools/_proto/", + strip_prefix = "src/", ) py_library( @@ -51,6 +51,7 @@ py_library( "grpc_tools/__init__.py", "grpc_tools/protoc.py", ], + data = [":well_known_protos"], imports = ["."], srcs_version = "PY2AND3", deps = [ @@ -58,5 +59,4 @@ py_library( "//src/python/grpcio/grpc:grpcio", "@com_google_protobuf//:protobuf_python", ], - data = [":well_known_protos"], ) diff --git a/tools/distrib/python/grpcio_tools/grpcio_tools.bzl b/tools/distrib/python/grpcio_tools/grpcio_tools.bzl index 8b6c121ffa6..603556c9d86 100644 --- a/tools/distrib/python/grpcio_tools/grpcio_tools.bzl +++ b/tools/distrib/python/grpcio_tools/grpcio_tools.bzl @@ -23,10 +23,12 @@ def _generate_copied_files_impl(ctx): if f.path.startswith("external"): external_separator = f.path.find("/") repository_separator = f.path.find("/", external_separator + 1) - destination_path = f.path[repository_separator+1:] + destination_path = f.path[repository_separator + 1:] if not destination_path.startswith(strip_prefix): fail("File '{}' did not start with '{}'.".format( - destination_path, strip_prefix)) + destination_path, + strip_prefix, + )) destination_path = dest + destination_path[len(strip_prefix):] destination_dir = destination_path.rfind("/") out_file = ctx.actions.declare_file(destination_path) @@ -35,12 +37,14 @@ def _generate_copied_files_impl(ctx): inputs = [f], outputs = [out_file], command = "mkdir -p {0} && cp {1} {2}".format( - out_file.dirname, f.path, out_file.path), + out_file.dirname, + f.path, + out_file.path, + ), ) return [DefaultInfo(files = depset(direct = outs))] - _generate_copied_files = rule( attrs = { "srcs": attr.label_list( @@ -52,12 +56,11 @@ _generate_copied_files = rule( ), "dest": attr.string( mandatory = True, - ) + ), }, implementation = _generate_copied_files_impl, ) - def internal_copied_filegroup(name, srcs, strip_prefix, dest): """Copies a file group to the current package. From 905bb9834a709c0abaa9998e484512e2cea8d326 Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Tue, 20 Oct 2020 13:31:36 -0700 Subject: [PATCH 6/6] Switch names around --- src/python/grpcio_tests/tests/unit/BUILD.bazel | 2 +- src/python/grpcio_tests/tests/unit/_dynamic_stubs_test.py | 2 +- .../tests/unit/data/foo/{baz.proto => bar_with_wkt.proto} | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) rename src/python/grpcio_tests/tests/unit/data/foo/{baz.proto => bar_with_wkt.proto} (92%) diff --git a/src/python/grpcio_tests/tests/unit/BUILD.bazel b/src/python/grpcio_tests/tests/unit/BUILD.bazel index 64dcab29615..9969729ce94 100644 --- a/src/python/grpcio_tests/tests/unit/BUILD.bazel +++ b/src/python/grpcio_tests/tests/unit/BUILD.bazel @@ -125,7 +125,7 @@ py2and3_test( srcs = ["_dynamic_stubs_test.py"], data = [ "data/foo/bar.proto", - "data/foo/baz.proto", + "data/foo/bar_with_wkt.proto", ], imports = ["../../"], main = "_dynamic_stubs_test.py", diff --git a/src/python/grpcio_tests/tests/unit/_dynamic_stubs_test.py b/src/python/grpcio_tests/tests/unit/_dynamic_stubs_test.py index ed107742b67..4645f0b633f 100644 --- a/src/python/grpcio_tests/tests/unit/_dynamic_stubs_test.py +++ b/src/python/grpcio_tests/tests/unit/_dynamic_stubs_test.py @@ -108,7 +108,7 @@ def _test_sunny_day(): def _test_well_known_types(): import grpc protos, services = grpc.protos_and_services( - os.path.join(_DATA_DIR, "foo", "baz.proto")) + os.path.join(_DATA_DIR, "foo", "bar_with_wkt.proto")) assert protos.BarMessage is not None assert services.BarStub is not None diff --git a/src/python/grpcio_tests/tests/unit/data/foo/baz.proto b/src/python/grpcio_tests/tests/unit/data/foo/bar_with_wkt.proto similarity index 92% rename from src/python/grpcio_tests/tests/unit/data/foo/baz.proto rename to src/python/grpcio_tests/tests/unit/data/foo/bar_with_wkt.proto index a2ca7ab524f..2fd1f437939 100644 --- a/src/python/grpcio_tests/tests/unit/data/foo/baz.proto +++ b/src/python/grpcio_tests/tests/unit/data/foo/bar_with_wkt.proto @@ -16,10 +16,11 @@ syntax = "proto3"; package tests.unit.data.foo.bar; -import "google/protobuf/wrappers.proto"; +import "google/protobuf/any.proto"; message BarMessage { string a = 1; + google.protobuf.Any b = 2; }; service Bar {