From 4798f4d33fe5e4b70cf2eedd553263486c6deb80 Mon Sep 17 00:00:00 2001 From: Kenneth Bandes Date: Thu, 28 Oct 2021 17:02:49 +0000 Subject: [PATCH 1/2] fix: methods returning Operation w/o operation_info are now allowed. --- gapic/schema/api.py | 4 +++ tests/unit/schema/test_api.py | 54 +++++++++++++++++++++++++++++++++-- 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/gapic/schema/api.py b/gapic/schema/api.py index cdfb5ca6e7..f8cbcb3a03 100644 --- a/gapic/schema/api.py +++ b/gapic/schema/api.py @@ -817,6 +817,10 @@ def _maybe_get_lro( # If the output type is google.longrunning.Operation, we use # a specialized object in its place. if meth_pb.output_type.endswith('google.longrunning.Operation'): + if not meth_pb.options.HasExtension(operations_pb2.operation_info): + # not a long running operation, although it returns an + # Operation message. + return None op = meth_pb.options.Extensions[operations_pb2.operation_info] if not op.response_type or not op.metadata_type: raise TypeError( diff --git a/tests/unit/schema/test_api.py b/tests/unit/schema/test_api.py index de705d88df..0192e37f47 100644 --- a/tests/unit/schema/test_api.py +++ b/tests/unit/schema/test_api.py @@ -944,19 +944,69 @@ def test_lro(): assert len(lro_proto.messages) == 1 -def test_lro_missing_annotation(): +def test_lro_operation_no_annotation(): + # A method that returns google.longrunning.Operation, + # but has no operation_info option, is treated as not lro. + # Set up a prior proto that mimics google/protobuf/empty.proto lro_proto = api.Proto.build(make_file_pb2( name='operations.proto', package='google.longrunning', messages=(make_message_pb2(name='Operation'),), ), file_to_generate=False, naming=make_naming()) - # Set up a method with an LRO but no annotation. + # Set up a method with return Operation, but with no annotation. + method_pb2 = descriptor_pb2.MethodDescriptorProto( + name='GetOperation', + input_type='google.example.v3.GetOperationRequest', + output_type='google.longrunning.Operation', + ) + + # Set up the service with an RPC. + service_pb = descriptor_pb2.ServiceDescriptorProto( + name='OperationService', + method=(method_pb2,), + ) + + # Set up the messages, including the annotated ones. + messages = ( + make_message_pb2(name='GetOperationRequest', fields=()), + ) + + # Finally, set up the file that encompasses these. + fdp = make_file_pb2( + package='google.example.v3', + messages=messages, + services=(service_pb,), + ) + + # Make the proto object. + proto = api.Proto.build(fdp, file_to_generate=True, prior_protos={ + 'google/longrunning/operations.proto': lro_proto, + }, naming=make_naming()) + + service = proto.services['google.example.v3.OperationService'] + method = service.methods['GetOperation'] + assert method.lro is None + + +def test_lro_bad_annotation(): + # Set up a prior proto that mimics google/protobuf/empty.proto + lro_proto = api.Proto.build(make_file_pb2( + name='operations.proto', package='google.longrunning', + messages=(make_message_pb2(name='Operation'),), + ), file_to_generate=False, naming=make_naming()) + + # Set up a method with an LRO and incomplete annotation. method_pb2 = descriptor_pb2.MethodDescriptorProto( name='AsyncDoThing', input_type='google.example.v3.AsyncDoThingRequest', output_type='google.longrunning.Operation', ) + method_pb2.options.Extensions[operations_pb2.operation_info].MergeFrom( + operations_pb2.OperationInfo( + response_type='google.example.v3.AsyncDoThingResponse', + ), + ) # Set up the service with an RPC. service_pb = descriptor_pb2.ServiceDescriptorProto( From 2410e4712d074e37207451f31e6769e8c9937090 Mon Sep 17 00:00:00 2001 From: Kenneth Bandes Date: Thu, 28 Oct 2021 17:21:19 +0000 Subject: [PATCH 2/2] fix: apply suggested grammar improvements. --- gapic/schema/api.py | 4 ++-- tests/unit/schema/test_api.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/gapic/schema/api.py b/gapic/schema/api.py index f8cbcb3a03..0e632ed217 100644 --- a/gapic/schema/api.py +++ b/gapic/schema/api.py @@ -818,8 +818,8 @@ def _maybe_get_lro( # a specialized object in its place. if meth_pb.output_type.endswith('google.longrunning.Operation'): if not meth_pb.options.HasExtension(operations_pb2.operation_info): - # not a long running operation, although it returns an - # Operation message. + # This is not a long running operation even though it returns + # an Operation. return None op = meth_pb.options.Extensions[operations_pb2.operation_info] if not op.response_type or not op.metadata_type: diff --git a/tests/unit/schema/test_api.py b/tests/unit/schema/test_api.py index 0192e37f47..2c139ce7f1 100644 --- a/tests/unit/schema/test_api.py +++ b/tests/unit/schema/test_api.py @@ -954,7 +954,7 @@ def test_lro_operation_no_annotation(): messages=(make_message_pb2(name='Operation'),), ), file_to_generate=False, naming=make_naming()) - # Set up a method with return Operation, but with no annotation. + # Set up a method that returns an Operation, but has no annotation. method_pb2 = descriptor_pb2.MethodDescriptorProto( name='GetOperation', input_type='google.example.v3.GetOperationRequest',