From 8d602bb60530a6cdc0af5d33537facdaf5f287da Mon Sep 17 00:00:00 2001 From: Bu Sun Kim Date: Mon, 1 Nov 2021 23:48:05 +0000 Subject: [PATCH 1/4] fix(snippetgen) fix streaming samples --- gapic/samplegen/samplegen.py | 10 ---------- gapic/templates/examples/feature_fragments.j2 | 17 +++++++++++++++-- gapic/templates/examples/sample.py.j2 | 2 +- ...logging_service_v2_tail_log_entries_async.py | 11 ++++++++++- ..._logging_service_v2_tail_log_entries_sync.py | 11 ++++++++++- 5 files changed, 36 insertions(+), 15 deletions(-) diff --git a/gapic/samplegen/samplegen.py b/gapic/samplegen/samplegen.py index 1538173ce9..9e5d0dd20e 100644 --- a/gapic/samplegen/samplegen.py +++ b/gapic/samplegen/samplegen.py @@ -551,16 +551,6 @@ def validate_and_transform_request( AttributeRequestSetup(**r_dup) # type: ignore ) - client_streaming_forms = { - types.CallingForm.RequestStreamingClient, - types.CallingForm.RequestStreamingBidi, - } - - if len(base_param_to_attrs) > 1 and calling_form in client_streaming_forms: - raise types.InvalidRequestSetup( - "Too many base parameters for client side streaming form" - ) - # We can only flatten a collection of request parameters if they're a # subset of the flattened fields of the method. flattenable = self.flattenable_fields >= set(base_param_to_attrs) diff --git a/gapic/templates/examples/feature_fragments.j2 b/gapic/templates/examples/feature_fragments.j2 index 7938a648b6..5fd500b115 100644 --- a/gapic/templates/examples/feature_fragments.j2 +++ b/gapic/templates/examples/feature_fragments.j2 @@ -153,7 +153,7 @@ with open({{ attr.input_parameter }}, "rb") as f: client = {{ module_name }}.{{ client_name }}() {% endmacro %} -{% macro render_request_setup(full_request, module_name, request_type) %} +{% macro render_request_setup(full_request, module_name, request_type, calling_form, calling_form_enum) %} # Initialize request argument(s) {% for parameter_block in full_request.request_list if parameter_block.body %} {% if parameter_block.pattern %} @@ -178,6 +178,19 @@ request = {{ module_name }}.{{ request_type.ident.name }}( {{ parameter.base }}={{ parameter.base if parameter.body else parameter.single.value }}, {% endfor %} ) +{# If client streaming, wrap the single request in a generator that produces 'requests' #} +{% if calling_form in [calling_form_enum.RequestStreamingBidi, + calling_form_enum.RequestStreamingClient] %} + +# This method expects an iterator which contains +# '{{module_name}}.{{ request_type.ident.name }}' objects +# Here we have create a generator that yields a single `request` for +# demonstrative purposes. +requests = [request] +def request_generator(): + for request in requests: + yield request +{% endif %} {% endif %} {% endmacro %} @@ -218,7 +231,7 @@ await{{ " "}} {%- endif -%} {% if calling_form in [calling_form_enum.RequestStreamingBidi, calling_form_enum.RequestStreamingClient] %} -client.{{ sample.rpc|snake_case }}([{{ render_request_params(sample.request.request_list)|trim }}]) +client.{{ sample.rpc|snake_case }}(requests=request_generator()) {% else %}{# TODO: deal with flattening #} {# TODO: set up client streaming once some questions are answered #} client.{{ sample.rpc|snake_case }}({{ render_request_params_unary(sample.request)|trim }}) diff --git a/gapic/templates/examples/sample.py.j2 b/gapic/templates/examples/sample.py.j2 index 54db08ca21..3191860c0c 100644 --- a/gapic/templates/examples/sample.py.j2 +++ b/gapic/templates/examples/sample.py.j2 @@ -35,7 +35,7 @@ from {{ sample.module_namespace|join(".") }} import {{ sample.module_name }} """{{ sample.description }}""" {{ frags.render_client_setup(sample.module_name, sample.client_name)|indent }} - {{ frags.render_request_setup(sample.request, sample.module_name, sample.request_type)|indent }} + {{ frags.render_request_setup(sample.request, sample.module_name, sample.request_type, calling_form, calling_form_enum)|indent }} {% with method_call = frags.render_method_call(sample, calling_form, calling_form_enum, sample.transport) %} {{ frags.render_calling_form(method_call, calling_form, calling_form_enum, sample.transport, sample.response)|indent -}} {% endwith %} diff --git a/tests/integration/goldens/logging/samples/generated_samples/logging_generated_logging_v2_logging_service_v2_tail_log_entries_async.py b/tests/integration/goldens/logging/samples/generated_samples/logging_generated_logging_v2_logging_service_v2_tail_log_entries_async.py index 38efe9e40d..1bf99b074a 100644 --- a/tests/integration/goldens/logging/samples/generated_samples/logging_generated_logging_v2_logging_service_v2_tail_log_entries_async.py +++ b/tests/integration/goldens/logging/samples/generated_samples/logging_generated_logging_v2_logging_service_v2_tail_log_entries_async.py @@ -38,8 +38,17 @@ async def sample_tail_log_entries(): resource_names=['resource_names_value_1', 'resource_names_value_2'], ) + # This method expects an iterator which contains + # 'logging_v2.TailLogEntriesRequest' objects + # Here we have create a generator that yields a single `request` for + # demonstrative purposes. + requests = [request] + def request_generator(): + for request in requests: + yield request + # Make the request - stream = await client.tail_log_entries([resource_names=['resource_names_value_1', 'resource_names_value_2']]) + stream = await client.tail_log_entries(requests=request_generator()) async for response in stream: print(response) diff --git a/tests/integration/goldens/logging/samples/generated_samples/logging_generated_logging_v2_logging_service_v2_tail_log_entries_sync.py b/tests/integration/goldens/logging/samples/generated_samples/logging_generated_logging_v2_logging_service_v2_tail_log_entries_sync.py index a533ee9ad6..b1c2f99f5e 100644 --- a/tests/integration/goldens/logging/samples/generated_samples/logging_generated_logging_v2_logging_service_v2_tail_log_entries_sync.py +++ b/tests/integration/goldens/logging/samples/generated_samples/logging_generated_logging_v2_logging_service_v2_tail_log_entries_sync.py @@ -38,8 +38,17 @@ def sample_tail_log_entries(): resource_names=['resource_names_value_1', 'resource_names_value_2'], ) + # This method expects an iterator which contains + # 'logging_v2.TailLogEntriesRequest' objects + # Here we have create a generator that yields a single `request` for + # demonstrative purposes. + requests = [request] + def request_generator(): + for request in requests: + yield request + # Make the request - stream = client.tail_log_entries([resource_names=['resource_names_value_1', 'resource_names_value_2']]) + stream = client.tail_log_entries(requests=request_generator()) for response in stream: print(response) From b6c56aa6d214c4c970a47e8bacdb7715def7d05c Mon Sep 17 00:00:00 2001 From: Bu Sun Kim Date: Tue, 2 Nov 2021 16:03:04 +0000 Subject: [PATCH 2/4] test: fix tests --- gapic/templates/examples/feature_fragments.j2 | 2 +- ...v2_logging_service_v2_tail_log_entries_async.py | 2 +- ..._v2_logging_service_v2_tail_log_entries_sync.py | 2 +- ...usca_v1_snippets_method_bidi_streaming_async.py | 11 ++++++++++- ...lusca_v1_snippets_method_bidi_streaming_sync.py | 11 ++++++++++- tests/unit/samplegen/test_template.py | 14 ++++++++------ 6 files changed, 31 insertions(+), 11 deletions(-) diff --git a/gapic/templates/examples/feature_fragments.j2 b/gapic/templates/examples/feature_fragments.j2 index 5fd500b115..acdcf0e9f2 100644 --- a/gapic/templates/examples/feature_fragments.j2 +++ b/gapic/templates/examples/feature_fragments.j2 @@ -184,7 +184,7 @@ request = {{ module_name }}.{{ request_type.ident.name }}( # This method expects an iterator which contains # '{{module_name}}.{{ request_type.ident.name }}' objects -# Here we have create a generator that yields a single `request` for +# Here we create a generator that yields a single `request` for # demonstrative purposes. requests = [request] def request_generator(): diff --git a/tests/integration/goldens/logging/samples/generated_samples/logging_generated_logging_v2_logging_service_v2_tail_log_entries_async.py b/tests/integration/goldens/logging/samples/generated_samples/logging_generated_logging_v2_logging_service_v2_tail_log_entries_async.py index 1bf99b074a..1c6e1db5be 100644 --- a/tests/integration/goldens/logging/samples/generated_samples/logging_generated_logging_v2_logging_service_v2_tail_log_entries_async.py +++ b/tests/integration/goldens/logging/samples/generated_samples/logging_generated_logging_v2_logging_service_v2_tail_log_entries_async.py @@ -40,7 +40,7 @@ async def sample_tail_log_entries(): # This method expects an iterator which contains # 'logging_v2.TailLogEntriesRequest' objects - # Here we have create a generator that yields a single `request` for + # Here we create a generator that yields a single `request` for # demonstrative purposes. requests = [request] def request_generator(): diff --git a/tests/integration/goldens/logging/samples/generated_samples/logging_generated_logging_v2_logging_service_v2_tail_log_entries_sync.py b/tests/integration/goldens/logging/samples/generated_samples/logging_generated_logging_v2_logging_service_v2_tail_log_entries_sync.py index b1c2f99f5e..e9a5dfc2ec 100644 --- a/tests/integration/goldens/logging/samples/generated_samples/logging_generated_logging_v2_logging_service_v2_tail_log_entries_sync.py +++ b/tests/integration/goldens/logging/samples/generated_samples/logging_generated_logging_v2_logging_service_v2_tail_log_entries_sync.py @@ -40,7 +40,7 @@ def sample_tail_log_entries(): # This method expects an iterator which contains # 'logging_v2.TailLogEntriesRequest' objects - # Here we have create a generator that yields a single `request` for + # Here we create a generator that yields a single `request` for # demonstrative purposes. requests = [request] def request_generator(): diff --git a/tests/snippetgen/goldens/mollusca_generated_mollusca_v1_snippets_method_bidi_streaming_async.py b/tests/snippetgen/goldens/mollusca_generated_mollusca_v1_snippets_method_bidi_streaming_async.py index 98190750d5..7e9605052e 100644 --- a/tests/snippetgen/goldens/mollusca_generated_mollusca_v1_snippets_method_bidi_streaming_async.py +++ b/tests/snippetgen/goldens/mollusca_generated_mollusca_v1_snippets_method_bidi_streaming_async.py @@ -38,8 +38,17 @@ async def sample_method_bidi_streaming(): my_string="my_string_value", ) + # This method expects an iterator which contains + # 'mollusca_v1.SignatureRequestOneRequiredField' objects + # Here we create a generator that yields a single `request` for + # demonstrative purposes. + requests = [request] + def request_generator(): + for request in requests: + yield request + # Make the request - stream = await client.method_bidi_streaming([my_string="my_string_value"]) + stream = await client.method_bidi_streaming(requests=request_generator()) async for response in stream: print(response) diff --git a/tests/snippetgen/goldens/mollusca_generated_mollusca_v1_snippets_method_bidi_streaming_sync.py b/tests/snippetgen/goldens/mollusca_generated_mollusca_v1_snippets_method_bidi_streaming_sync.py index de6ffb254d..269fe19732 100644 --- a/tests/snippetgen/goldens/mollusca_generated_mollusca_v1_snippets_method_bidi_streaming_sync.py +++ b/tests/snippetgen/goldens/mollusca_generated_mollusca_v1_snippets_method_bidi_streaming_sync.py @@ -38,8 +38,17 @@ def sample_method_bidi_streaming(): my_string="my_string_value", ) + # This method expects an iterator which contains + # 'mollusca_v1.SignatureRequestOneRequiredField' objects + # Here we create a generator that yields a single `request` for + # demonstrative purposes. + requests = [request] + def request_generator(): + for request in requests: + yield request + # Make the request - stream = client.method_bidi_streaming([my_string="my_string_value"]) + stream = client.method_bidi_streaming(requests=request_generator()) for response in stream: print(response) diff --git a/tests/unit/samplegen/test_template.py b/tests/unit/samplegen/test_template.py index 3d1b3cff26..763c850cc5 100644 --- a/tests/unit/samplegen/test_template.py +++ b/tests/unit/samplegen/test_template.py @@ -200,7 +200,7 @@ def test_render_request_unflattened(): check_template( ''' {% import "feature_fragments.j2" as frags %} - {{ frags.render_request_setup(request, module_name, request_type) }} + {{ frags.render_request_setup(request, module_name, request_type, calling_form, calling_form_enum) }} ''', ''' # Initialize request argument(s) @@ -289,7 +289,9 @@ def test_render_request_unflattened(): ) }, ident=common_types.DummyIdent(name="CreateMolluscRequest") - ) + ), + calling_form_enum=CallingForm, + calling_form=CallingForm.Request, ) @@ -1012,7 +1014,7 @@ def test_render_method_call_bidi(): calling_form, calling_form_enum, transport) }} ''', ''' - client.categorize_mollusc([video]) + client.categorize_mollusc(requests=request_generator()) ''', request=samplegen.FullRequest( request_list=[ @@ -1037,7 +1039,7 @@ def test_render_method_call_bidi_async(): calling_form, calling_form_enum, transport) }} ''', ''' - await client.categorize_mollusc([video]) + await client.categorize_mollusc(requests=request_generator()) ''', request=samplegen.FullRequest( request_list=[ @@ -1062,7 +1064,7 @@ def test_render_method_call_client(): calling_form, calling_form_enum, transport) }} ''', ''' - client.categorize_mollusc([video]) + client.categorize_mollusc(requests=request_generator()) ''', request=samplegen.FullRequest( request_list=[ @@ -1087,7 +1089,7 @@ def test_render_method_call_client_async(): calling_form, calling_form_enum, transport) }} ''', ''' - await client.categorize_mollusc([video]) + await client.categorize_mollusc(requests=request_generator()) ''', request=samplegen.FullRequest( request_list=[ From 06407c7abe2ab41ce9043ab266cd413cc1524ca9 Mon Sep 17 00:00:00 2001 From: Bu Sun Kim Date: Fri, 5 Nov 2021 20:38:54 +0000 Subject: [PATCH 3/4] test: delete obsolete tests, ignore snippetgen goldens for autopep8 --- .github/workflows/tests.yaml | 2 +- DEVELOPMENT.md | 4 +- gapic/samplegen/samplegen.py | 4 +- gapic/templates/examples/feature_fragments.j2 | 2 + tests/unit/samplegen/test_samplegen.py | 51 ------------------- 5 files changed, 6 insertions(+), 57 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 33f2264da6..3e6494bf2c 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -355,4 +355,4 @@ jobs: python -m pip install autopep8 - name: Check diff run: | - find gapic tests -name "*.py" -not -path 'tests/integration/goldens/*' | xargs autopep8 --diff --exit-code + find gapic tests -name "*.py" -not -path 'tests/integration/goldens/*' -not -path 'tests/snippetgen/goldens/*' | xargs autopep8 --diff --exit-code diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 57d048b62d..2079f744be 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -26,13 +26,13 @@ Execute unit tests by running one of the sessions prefixed with `unit-`. - Lint sources by running `autopep8`. The specific command is the following. ``` - find gapic tests -name "*.py" -not -path 'tests/integration/goldens/*' | xargs autopep8 --diff --exit-code + find gapic tests -name "*.py" -not -path 'tests/**/goldens/*' | xargs autopep8 --diff --exit-code ``` - Format sources in place: ``` - find gapic tests -name "*.py" -not -path 'tests/integration/goldens/*' | xargs autopep8 --in-place + find gapic tests -name "*.py" -not -path 'tests/**/goldens/*' | xargs autopep8 --in-place ``` ## Integration Tests diff --git a/gapic/samplegen/samplegen.py b/gapic/samplegen/samplegen.py index 9e5d0dd20e..88d3702429 100644 --- a/gapic/samplegen/samplegen.py +++ b/gapic/samplegen/samplegen.py @@ -469,9 +469,7 @@ def validate_and_transform_request( Raises: InvalidRequestSetup: If a dict in the request lacks a "field" key, - a "value" key, if there is an unexpected keyword, - or if more than one base parameter is given for - a client-side streaming calling form. + a "value" key or if there is an unexpected keyword. BadAttributeLookup: If a request field refers to a non-existent field in the request message type. ResourceRequestMismatch: If a request attempts to describe both diff --git a/gapic/templates/examples/feature_fragments.j2 b/gapic/templates/examples/feature_fragments.j2 index acdcf0e9f2..0f2e09ac14 100644 --- a/gapic/templates/examples/feature_fragments.j2 +++ b/gapic/templates/examples/feature_fragments.j2 @@ -178,6 +178,8 @@ request = {{ module_name }}.{{ request_type.ident.name }}( {{ parameter.base }}={{ parameter.base if parameter.body else parameter.single.value }}, {% endfor %} ) +{# Note: This template assumes only one request needs to be sent. When samples accept +configs the client streaming logic should be modified to allow 2+ request objects. #} {# If client streaming, wrap the single request in a generator that produces 'requests' #} {% if calling_form in [calling_form_enum.RequestStreamingBidi, calling_form_enum.RequestStreamingClient] %} diff --git a/tests/unit/samplegen/test_samplegen.py b/tests/unit/samplegen/test_samplegen.py index b44f34c13e..8677f3e8be 100644 --- a/tests/unit/samplegen/test_samplegen.py +++ b/tests/unit/samplegen/test_samplegen.py @@ -1350,57 +1350,6 @@ def test_validate_request_reserved_input_param(dummy_api_schema): ) -def test_single_request_client_streaming(dummy_api_schema, - calling_form=types.CallingForm.RequestStreamingClient): - # Each API client method really only takes one parameter: - # either a single protobuf message or an iterable of protobuf messages. - # With unary request methods, python lets us describe attributes as positional - # and keyword parameters, which simplifies request construction. - # The 'base' in the transformed request refers to an attribute, and the - # 'field's refer to sub-attributes. - # Client streaming and bidirectional streaming methods can't use this notation, - # and generate an exception if there is more than one 'base'. - input_type = DummyMessage( - fields={ - "cephalopod": DummyField( - message=DummyMessage( - fields={ - "order": DummyField( - message=DummyMessage(type="ORDER_TYPE") - ) - }, - type="CEPHALOPOD_TYPE" - ) - ), - "gastropod": DummyField( - message=DummyMessage( - fields={ - "order": DummyField( - message=DummyMessage(type="ORDER_TYPE") - ) - }, - type="GASTROPOD_TYPE" - ) - ) - }, - type="MOLLUSC_TYPE" - ) - v = samplegen.Validator(DummyMethod(input=input_type), dummy_api_schema) - with pytest.raises(types.InvalidRequestSetup): - v.validate_and_transform_request( - types.CallingForm.RequestStreamingClient, - [ - {"field": "cephalopod.order", "value": "cephalopoda"}, - {"field": "gastropod.order", "value": "pulmonata"}, - ], - ) - - -def test_single_request_bidi_streaming(): - test_single_request_client_streaming( - types.CallingForm.RequestStreamingBidi) - - def test_validate_request_calling_form(): assert ( types.CallingForm.method_default(DummyMethod(lro=True)) From 80d33cbbdce4ed14e99dfb39960d3d4dede9db53 Mon Sep 17 00:00:00 2001 From: Bu Sun Kim Date: Fri, 5 Nov 2021 20:39:29 +0000 Subject: [PATCH 4/4] chore: update .github --- .github/workflows/tests.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 3e6494bf2c..61f120912f 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -355,4 +355,4 @@ jobs: python -m pip install autopep8 - name: Check diff run: | - find gapic tests -name "*.py" -not -path 'tests/integration/goldens/*' -not -path 'tests/snippetgen/goldens/*' | xargs autopep8 --diff --exit-code + find gapic tests -name "*.py" -not -path 'tests/**/goldens/*' | xargs autopep8 --diff --exit-code