From d6498800441e68e22854ee7daf94cadb709ffac9 Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Fri, 30 Aug 2019 16:39:02 -0700 Subject: [PATCH 1/4] Add strip_prefix to python protoc plugin and py_grpc_library --- bazel/python_rules.bzl | 11 +++++- examples/python/debug/get_stats.py | 9 ++--- src/compiler/generator_helpers.h | 9 ++--- src/compiler/python_generator.cc | 35 ++++++++++++++++--- .../grpc_channelz/v1/BUILD.bazel | 1 + .../grpc_channelz/v1/channelz.py | 9 ++--- .../grpc_health/v1/BUILD.bazel | 1 + .../grpc_health/v1/health.py | 9 ++--- .../grpc_reflection/v1alpha/BUILD.bazel | 1 + .../grpc_reflection/v1alpha/reflection.py | 11 ++---- .../tests/channelz/_channelz_servicer_test.py | 12 ++----- .../health_check/_health_servicer_test.py | 12 ++----- .../reflection/_reflection_servicer_test.py | 12 ++----- 13 files changed, 66 insertions(+), 66 deletions(-) diff --git a/bazel/python_rules.bzl b/bazel/python_rules.bzl index 12f51f8b172..0cdd2b9739c 100644 --- a/bazel/python_rules.bzl +++ b/bazel/python_rules.bzl @@ -93,11 +93,13 @@ def _generate_pb2_grpc_src_impl(context): proto_root = get_proto_root(context.label.workspace_root) out_files = declare_out_files(protos, context, _GENERATED_GRPC_PROTO_FORMAT) + plugin_flags = ["grpc_2_0"] + context.attr.strip_prefixes + arguments = [] tools = [context.executable._protoc, context.executable._plugin] arguments += get_plugin_args( context.executable._plugin, - [], + plugin_flags, context.genfiles_dir.path, False, ) @@ -127,6 +129,7 @@ _generate_pb2_grpc_src = rule( allow_empty = False, providers = [ProtoInfo], ), + "strip_prefixes": attr.string_list(), "_plugin": attr.label( executable = True, providers = ["files_to_run"], @@ -147,6 +150,7 @@ def py_grpc_library( name, srcs, deps, + strip_prefixes = [], **kwargs): """Generate python code for gRPC services defined in a protobuf. @@ -156,6 +160,10 @@ def py_grpc_library( schema of the service. deps: (List of `labels`) a single py_proto_library target for the proto_library in `srcs`. + strip_prefixes: (List of `strings`) If provided, this prefix will be + stripped from the beginning of foo_pb2 modules imported by the + generated stubs. This is useful in combination with the `imports` + attribute of the `py_library` rule. """ codegen_grpc_target = "_{}_grpc_codegen".format(name) if len(srcs) != 1: @@ -167,6 +175,7 @@ def py_grpc_library( _generate_pb2_grpc_src( name = codegen_grpc_target, deps = srcs, + strip_prefixes = strip_prefixes, **kwargs ) diff --git a/examples/python/debug/get_stats.py b/examples/python/debug/get_stats.py index 2da51d0efcf..d8cb08df55e 100644 --- a/examples/python/debug/get_stats.py +++ b/examples/python/debug/get_stats.py @@ -21,13 +21,8 @@ import logging import argparse import grpc -# TODO(https://github.com/grpc/grpc/issues/19863): Remove. -try: - from src.python.grpcio_channelz.grpc_channelz.v1 import channelz_pb2 - from src.python.grpcio_channelz.grpc_channelz.v1 import channelz_pb2_grpc -except ImportError: - from grpc_channelz.v1 import channelz_pb2 - from grpc_channelz.v1 import channelz_pb2_grpc +from grpc_channelz.v1 import channelz_pb2 +from grpc_channelz.v1 import channelz_pb2_grpc def run(addr): diff --git a/src/compiler/generator_helpers.h b/src/compiler/generator_helpers.h index 747096f0657..0f261fb5a83 100644 --- a/src/compiler/generator_helpers.h +++ b/src/compiler/generator_helpers.h @@ -168,10 +168,11 @@ inline MethodType GetMethodType( inline void Split(const grpc::string& s, char delim, std::vector* append_to) { - std::istringstream iss(s); - grpc::string piece; - while (std::getline(iss, piece)) { - append_to->push_back(piece); + auto current = s.begin(); + while (current <= s.end()) { + auto next = std::find(current, s.end(), delim); + append_to->emplace_back(current, next); + current = next + 1; } } diff --git a/src/compiler/python_generator.cc b/src/compiler/python_generator.cc index 705aef1c884..7afb2c532e0 100644 --- a/src/compiler/python_generator.cc +++ b/src/compiler/python_generator.cc @@ -756,6 +756,28 @@ static bool GenerateGrpc(GeneratorContext* context, PrivateGenerator& generator, } } +static bool ParseParameters(const grpc::string& parameter, + grpc::string* grpc_version, + std::vector* strip_prefixes, + grpc::string* error) { + std::vector comma_delimited_parameters; + grpc_generator::Split(parameter, ',', &comma_delimited_parameters); + if (comma_delimited_parameters.empty()) { + *grpc_version = "grpc_2_0"; + } else if (comma_delimited_parameters.size() == 1) { + *grpc_version = comma_delimited_parameters[0]; + } else if (comma_delimited_parameters.size() == 2) { + *grpc_version = comma_delimited_parameters[0]; + std::copy(comma_delimited_parameters.begin() + 1, + comma_delimited_parameters.end(), + std::back_inserter(*strip_prefixes)); + } else { + *error = "--grpc_python_out received too many comma-delimited parameters."; + return false; + } + return true; +} + bool PythonGrpcGenerator::Generate(const FileDescriptor* file, const grpc::string& parameter, GeneratorContext* context, @@ -778,14 +800,19 @@ bool PythonGrpcGenerator::Generate(const FileDescriptor* file, generator_file_name = file->name(); ProtoBufFile pbfile(file); - PrivateGenerator generator(config_, &pbfile); - if (parameter == "" || parameter == "grpc_2_0") { + grpc::string grpc_version; + GeneratorConfiguration extended_config(config_); + bool success = ParseParameters(parameter, &grpc_version, + &(extended_config.prefixes_to_filter), error); + PrivateGenerator generator(extended_config, &pbfile); + if (!success) return false; + if (grpc_version == "grpc_2_0") { return GenerateGrpc(context, generator, pb2_grpc_file_name, true); - } else if (parameter == "grpc_1_0") { + } else if (grpc_version == "grpc_1_0") { return GenerateGrpc(context, generator, pb2_grpc_file_name, true) && GenerateGrpc(context, generator, pb2_file_name, false); } else { - *error = "Invalid parameter '" + parameter + "'."; + *error = "Invalid grpc version '" + grpc_version + "'."; return false; } } diff --git a/src/python/grpcio_channelz/grpc_channelz/v1/BUILD.bazel b/src/python/grpcio_channelz/grpc_channelz/v1/BUILD.bazel index 5f4e512e9f4..c034297ff2d 100644 --- a/src/python/grpcio_channelz/grpc_channelz/v1/BUILD.bazel +++ b/src/python/grpcio_channelz/grpc_channelz/v1/BUILD.bazel @@ -10,6 +10,7 @@ py_grpc_library( name = "channelz_py_pb2_grpc", srcs = ["//src/proto/grpc/channelz:channelz_proto_descriptors"], deps = [":channelz_py_pb2"], + strip_prefixes = ["src.python.grpcio_channelz."], ) py_library( diff --git a/src/python/grpcio_channelz/grpc_channelz/v1/channelz.py b/src/python/grpcio_channelz/grpc_channelz/v1/channelz.py index 965797f89ec..00eca311dc1 100644 --- a/src/python/grpcio_channelz/grpc_channelz/v1/channelz.py +++ b/src/python/grpcio_channelz/grpc_channelz/v1/channelz.py @@ -16,13 +16,8 @@ import grpc from grpc._cython import cygrpc -# TODO(https://github.com/grpc/grpc/issues/19863): Remove. -try: - from src.python.grpcio_channelz.grpc_channelz.v1 import channelz_pb2 as _channelz_pb2 - from src.python.grpcio_channelz.grpc_channelz.v1 import channelz_pb2_grpc as _channelz_pb2_grpc -except ImportError: - import grpc_channelz.v1.channelz_pb2 as _channelz_pb2 - import grpc_channelz.v1.channelz_pb2_grpc as _channelz_pb2_grpc +import grpc_channelz.v1.channelz_pb2 as _channelz_pb2 +import grpc_channelz.v1.channelz_pb2_grpc as _channelz_pb2_grpc from google.protobuf import json_format diff --git a/src/python/grpcio_health_checking/grpc_health/v1/BUILD.bazel b/src/python/grpcio_health_checking/grpc_health/v1/BUILD.bazel index 62a44df7707..a3e2c7dfe3d 100644 --- a/src/python/grpcio_health_checking/grpc_health/v1/BUILD.bazel +++ b/src/python/grpcio_health_checking/grpc_health/v1/BUILD.bazel @@ -10,6 +10,7 @@ py_grpc_library( name = "health_py_pb2_grpc", srcs = ["//src/proto/grpc/health/v1:health_proto_descriptor",], deps = [":health_py_pb2"], + strip_prefixes = ["src.python.grpcio_health_checking."], ) py_library( diff --git a/src/python/grpcio_health_checking/grpc_health/v1/health.py b/src/python/grpcio_health_checking/grpc_health/v1/health.py index a0d55570990..15494fafdbc 100644 --- a/src/python/grpcio_health_checking/grpc_health/v1/health.py +++ b/src/python/grpcio_health_checking/grpc_health/v1/health.py @@ -18,13 +18,8 @@ import threading import grpc -# TODO(https://github.com/grpc/grpc/issues/19863): Remove. -try: - from src.python.grpcio_health_checking.grpc_health.v1 import health_pb2 as _health_pb2 - from src.python.grpcio_health_checking.grpc_health.v1 import health_pb2_grpc as _health_pb2_grpc -except ImportError: - from grpc_health.v1 import health_pb2 as _health_pb2 - from grpc_health.v1 import health_pb2_grpc as _health_pb2_grpc +from grpc_health.v1 import health_pb2 as _health_pb2 +from grpc_health.v1 import health_pb2_grpc as _health_pb2_grpc SERVICE_NAME = _health_pb2.DESCRIPTOR.services_by_name['Health'].full_name diff --git a/src/python/grpcio_reflection/grpc_reflection/v1alpha/BUILD.bazel b/src/python/grpcio_reflection/grpc_reflection/v1alpha/BUILD.bazel index 10077fd9568..bad54d06c63 100644 --- a/src/python/grpcio_reflection/grpc_reflection/v1alpha/BUILD.bazel +++ b/src/python/grpcio_reflection/grpc_reflection/v1alpha/BUILD.bazel @@ -12,6 +12,7 @@ py_grpc_library( name = "reflection_py_pb2_grpc", srcs = ["//src/proto/grpc/reflection/v1alpha:reflection_proto_descriptor",], deps = ["reflection_py_pb2"], + strip_prefixes = ["src.python.grpcio_reflection."], ) py_library( diff --git a/src/python/grpcio_reflection/grpc_reflection/v1alpha/reflection.py b/src/python/grpcio_reflection/grpc_reflection/v1alpha/reflection.py index 61153d9d625..6df1a364269 100644 --- a/src/python/grpcio_reflection/grpc_reflection/v1alpha/reflection.py +++ b/src/python/grpcio_reflection/grpc_reflection/v1alpha/reflection.py @@ -17,15 +17,8 @@ import grpc from google.protobuf import descriptor_pb2 from google.protobuf import descriptor_pool -# TODO(https://github.com/grpc/grpc/issues/19863): Remove. -try: - from src.python.grpcio_reflection.grpc_reflection.v1alpha \ - import reflection_pb2 as _reflection_pb2 - from src.python.grpcio_reflection.grpc_reflection.v1alpha \ - import reflection_pb2_grpc as _reflection_pb2_grpc -except ImportError: - from grpc_reflection.v1alpha import reflection_pb2 as _reflection_pb2 - from grpc_reflection.v1alpha import reflection_pb2_grpc as _reflection_pb2_grpc +from grpc_reflection.v1alpha import reflection_pb2 as _reflection_pb2 +from grpc_reflection.v1alpha import reflection_pb2_grpc as _reflection_pb2_grpc _POOL = descriptor_pool.Default() SERVICE_NAME = _reflection_pb2.DESCRIPTOR.services_by_name[ diff --git a/src/python/grpcio_tests/tests/channelz/_channelz_servicer_test.py b/src/python/grpcio_tests/tests/channelz/_channelz_servicer_test.py index f2357a4e4a3..48d25e99e35 100644 --- a/src/python/grpcio_tests/tests/channelz/_channelz_servicer_test.py +++ b/src/python/grpcio_tests/tests/channelz/_channelz_servicer_test.py @@ -19,15 +19,9 @@ from concurrent import futures import grpc -# TODO(https://github.com/grpc/grpc/issues/19863): Remove. -try: - from src.python.grpcio_channelz.grpc_channelz.v1 import channelz - from src.python.grpcio_channelz.grpc_channelz.v1 import channelz_pb2 - from src.python.grpcio_channelz.grpc_channelz.v1 import channelz_pb2_grpc -except ImportError: - from grpc_channelz.v1 import channelz - from grpc_channelz.v1 import channelz_pb2 - from grpc_channelz.v1 import channelz_pb2_grpc +from grpc_channelz.v1 import channelz +from grpc_channelz.v1 import channelz_pb2 +from grpc_channelz.v1 import channelz_pb2_grpc from tests.unit import test_common from tests.unit.framework.common import test_constants diff --git a/src/python/grpcio_tests/tests/health_check/_health_servicer_test.py b/src/python/grpcio_tests/tests/health_check/_health_servicer_test.py index 428806c71b0..8faffb448c3 100644 --- a/src/python/grpcio_tests/tests/health_check/_health_servicer_test.py +++ b/src/python/grpcio_tests/tests/health_check/_health_servicer_test.py @@ -20,15 +20,9 @@ import unittest import grpc -# TODO(https://github.com/grpc/grpc/issues/19863): Remove. -try: - from src.python.grpcio_health_checking.grpc_health.v1 import health - from src.python.grpcio_health_checking.grpc_health.v1 import health_pb2 - from src.python.grpcio_health_checking.grpc_health.v1 import health_pb2_grpc -except ImportError: - from grpc_health.v1 import health - from grpc_health.v1 import health_pb2 - from grpc_health.v1 import health_pb2_grpc +from grpc_health.v1 import health +from grpc_health.v1 import health_pb2 +from grpc_health.v1 import health_pb2_grpc from tests.unit import test_common from tests.unit import thread_pool diff --git a/src/python/grpcio_tests/tests/reflection/_reflection_servicer_test.py b/src/python/grpcio_tests/tests/reflection/_reflection_servicer_test.py index e45f98abec6..169e55022da 100644 --- a/src/python/grpcio_tests/tests/reflection/_reflection_servicer_test.py +++ b/src/python/grpcio_tests/tests/reflection/_reflection_servicer_test.py @@ -17,15 +17,9 @@ import unittest import grpc -# TODO(https://github.com/grpc/grpc/issues/19863): Remove. -try: - from src.python.grpcio_reflection.grpc_reflection.v1alpha import reflection - from src.python.grpcio_reflection.grpc_reflection.v1alpha import reflection_pb2 - from src.python.grpcio_reflection.grpc_reflection.v1alpha import reflection_pb2_grpc -except ImportError: - from grpc_reflection.v1alpha import reflection - from grpc_reflection.v1alpha import reflection_pb2 - from grpc_reflection.v1alpha import reflection_pb2_grpc +from grpc_reflection.v1alpha import reflection +from grpc_reflection.v1alpha import reflection_pb2 +from grpc_reflection.v1alpha import reflection_pb2_grpc from google.protobuf import descriptor_pool from google.protobuf import descriptor_pb2 From 358676db444d84db7412c695101c6b2f0e9774fd Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Fri, 30 Aug 2019 17:53:41 -0700 Subject: [PATCH 2/4] Alright. We'll do our own thing then --- src/compiler/generator_helpers.h | 9 ++++----- src/compiler/python_generator.cc | 2 +- src/compiler/python_generator_helpers.h | 10 ++++++++++ 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/compiler/generator_helpers.h b/src/compiler/generator_helpers.h index 0f261fb5a83..747096f0657 100644 --- a/src/compiler/generator_helpers.h +++ b/src/compiler/generator_helpers.h @@ -168,11 +168,10 @@ inline MethodType GetMethodType( inline void Split(const grpc::string& s, char delim, std::vector* append_to) { - auto current = s.begin(); - while (current <= s.end()) { - auto next = std::find(current, s.end(), delim); - append_to->emplace_back(current, next); - current = next + 1; + std::istringstream iss(s); + grpc::string piece; + while (std::getline(iss, piece)) { + append_to->push_back(piece); } } diff --git a/src/compiler/python_generator.cc b/src/compiler/python_generator.cc index 7afb2c532e0..0e76c95aed4 100644 --- a/src/compiler/python_generator.cc +++ b/src/compiler/python_generator.cc @@ -761,7 +761,7 @@ static bool ParseParameters(const grpc::string& parameter, std::vector* strip_prefixes, grpc::string* error) { std::vector comma_delimited_parameters; - grpc_generator::Split(parameter, ',', &comma_delimited_parameters); + grpc_python_generator::Split(parameter, ',', &comma_delimited_parameters); if (comma_delimited_parameters.empty()) { *grpc_version = "grpc_2_0"; } else if (comma_delimited_parameters.size() == 1) { diff --git a/src/compiler/python_generator_helpers.h b/src/compiler/python_generator_helpers.h index 171dd730a24..862292db475 100644 --- a/src/compiler/python_generator_helpers.h +++ b/src/compiler/python_generator_helpers.h @@ -136,6 +136,16 @@ StringVector get_all_comments(const DescriptorType* descriptor) { return comments; } +inline void Split(const grpc::string& s, char delim, + std::vector* append_to) { + auto current = s.begin(); + while (current <= s.end()) { + auto next = std::find(current, s.end(), delim); + append_to->emplace_back(current, next); + current = next + 1; + } +} + } // namespace } // namespace grpc_python_generator From e6fe85bc2ddb12bf2960fd27dc6f90a0e29d3ec5 Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Fri, 30 Aug 2019 18:20:49 -0700 Subject: [PATCH 3/4] Properly handle default case --- src/compiler/python_generator.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/compiler/python_generator.cc b/src/compiler/python_generator.cc index 0e76c95aed4..f553d9e7a3c 100644 --- a/src/compiler/python_generator.cc +++ b/src/compiler/python_generator.cc @@ -762,7 +762,8 @@ static bool ParseParameters(const grpc::string& parameter, grpc::string* error) { std::vector comma_delimited_parameters; grpc_python_generator::Split(parameter, ',', &comma_delimited_parameters); - if (comma_delimited_parameters.empty()) { + if (comma_delimited_parameters.size() == 1 && + comma_delimited_parameters.empty()) { *grpc_version = "grpc_2_0"; } else if (comma_delimited_parameters.size() == 1) { *grpc_version = comma_delimited_parameters[0]; From 88aef7cd75bcdb703d041317a5b719619fc71da8 Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Tue, 3 Sep 2019 17:12:28 -0700 Subject: [PATCH 4/4] Fix default case --- src/compiler/python_generator.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compiler/python_generator.cc b/src/compiler/python_generator.cc index f553d9e7a3c..8935a516f8a 100644 --- a/src/compiler/python_generator.cc +++ b/src/compiler/python_generator.cc @@ -763,7 +763,7 @@ static bool ParseParameters(const grpc::string& parameter, std::vector comma_delimited_parameters; grpc_python_generator::Split(parameter, ',', &comma_delimited_parameters); if (comma_delimited_parameters.size() == 1 && - comma_delimited_parameters.empty()) { + comma_delimited_parameters[0].empty()) { *grpc_version = "grpc_2_0"; } else if (comma_delimited_parameters.size() == 1) { *grpc_version = comma_delimited_parameters[0];