Skip to content

Commit

Permalink
fix(snippetgen): fix client streaming samples (#1061)
Browse files Browse the repository at this point in the history
Fixes #1014 and unblocks #1043.

NOTE: Some real world APIs expect the first request to pass a config (example) so the generated samples will not work out of the box. This will be addressed when the new sample config language is sorted out.
  • Loading branch information
busunkim96 authored Nov 8, 2021
1 parent a916186 commit 64b9ad6
Show file tree
Hide file tree
Showing 11 changed files with 70 additions and 80 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -359,4 +359,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/**/goldens/*' | xargs autopep8 --diff --exit-code
4 changes: 2 additions & 2 deletions DEVELOPMENT.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 1 addition & 13 deletions gapic/samplegen/samplegen.py
Original file line number Diff line number Diff line change
Expand Up @@ -466,9 +466,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
Expand Down Expand Up @@ -548,16 +546,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)
Expand Down
19 changes: 17 additions & 2 deletions gapic/templates/examples/feature_fragments.j2
Original file line number Diff line number Diff line change
Expand Up @@ -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 %}
Expand All @@ -179,6 +179,21 @@ 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] %}

# This method expects an iterator which contains
# '{{module_name}}.{{ request_type.ident.name }}' 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
{% endif %}
{% endif %}
{% endmacro %}

Expand Down Expand Up @@ -219,7 +234,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 }})
Expand Down
2 changes: 1 addition & 1 deletion gapic/templates/examples/sample.py.j2
Original file line number Diff line number Diff line change
Expand Up @@ -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 %}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 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)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 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)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
51 changes: 0 additions & 51 deletions tests/unit/samplegen/test_samplegen.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
14 changes: 8 additions & 6 deletions tests/unit/samplegen/test_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -289,7 +289,9 @@ def test_render_request_unflattened():
)
},
ident=common_types.DummyIdent(name="CreateMolluscRequest")
)
),
calling_form_enum=CallingForm,
calling_form=CallingForm.Request,
)


Expand Down Expand Up @@ -1015,7 +1017,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=[
Expand All @@ -1040,7 +1042,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=[
Expand All @@ -1065,7 +1067,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=[
Expand All @@ -1090,7 +1092,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=[
Expand Down

0 comments on commit 64b9ad6

Please sign in to comment.