Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: cater for empty async call #1997

Merged
merged 2 commits into from
Mar 22, 2024
Merged

fix: cater for empty async call #1997

merged 2 commits into from
Mar 22, 2024

Conversation

parthea
Copy link
Contributor

@parthea parthea commented Mar 22, 2024

This PR fixes the issue in the stack trace below seen in cl/618127799 which is similar to #791 but for the async client

            # Certain fields should be provided within the metadata header;
            # add these here.
            metadata = tuple(metadata) + (
                gapic_v1.routing_header.to_grpc_metadata((
>                   ("resource", request.resource),
                )),
            )
E           AttributeError: 'NoneType' object has no attribute 'resource'

@product-auto-label product-auto-label bot added the size: xs Pull request size is extra small. label Mar 22, 2024
@parthea parthea marked this pull request as ready for review March 22, 2024 15:24
@parthea parthea requested a review from a team as a code owner March 22, 2024 15:24
@parthea parthea added the owlbot:run Add this label to trigger the Owlbot post processor. label Mar 22, 2024
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Mar 22, 2024
elif not request:
request = {{ method.input.ident }}({% if method.input.ident.package != method.ident.package %}{% for f in method.flattened_fields.values() %}{{ f.name }}={{ f.name }}, {% endfor %}{% endif %})
{% endif %}{# Cross-package req and flattened fields #}
request = {{ method.input.ident }}({% if method.flattened_fields %}{% for f in method.flattened_fields.values() %}{{ f.name }}={{ f.name }}, {% endfor %}{% endif %})
{% else %}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need this j2 else case here?

Copy link
Contributor Author

@parthea parthea Mar 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the else case is needed when{% if method.input.ident.package != method.ident.package %} is false and also exists in the sync version here

{% else %}
# Minor optimization to avoid making a copy if the user passes
# in a {{ method.input.ident }}.
# There's no risk of modifying the input as we've already verified
# there are no flattened fields.
if not isinstance(request, {{ method.input.ident }}):
request = {{ method.input.ident }}(request)
{% endif %}{# different request package #}

@parthea parthea merged commit 801eedb into main Mar 22, 2024
67 checks passed
@parthea parthea deleted the add-null-request-async-client branch March 22, 2024 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: xs Pull request size is extra small.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants