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

Missing default deadlines in generated stubs #1477

Closed
igorbernstein2 opened this issue Oct 20, 2022 · 1 comment · Fixed by #1480
Closed

Missing default deadlines in generated stubs #1477

igorbernstein2 opened this issue Oct 20, 2022 · 1 comment · Fixed by #1480

Comments

@igorbernstein2
Copy link

Code example

from google.cloud.bigtable_admin_v2 import BigtableTableAdminClient
from google.cloud.bigtable_admin_v2.types import ModifyColumnFamiliesRequest, ColumnFamily,GcRule

c = BigtableTableAdminClient()
c.modify_column_families(
    ModifyColumnFamiliesRequest(
        name="projects/my-project/instances/my-instance/tables/my-table",
        modifications=[
            ModifyColumnFamiliesRequest.Modification(
                id="family",
                create=ColumnFamily(
                    gc_rule=GcRule(
                        max_num_versions=1
                    )
                )
            )
        ]
    )
)

This should send the RPC using the default deadline configured here in service json file, which generates the defaults base.py:
https://github.com/googleapis/python-bigtable/blob/0841824c7b9539df2530370ea128c140eda0f2b2/google/cloud/bigtable_admin_v2/services/bigtable_table_admin/transports/base.py#L199-L203

However this deadline is not respected. I believe the problem comes from the generator using None as the default in the stub:

https://github.com/googleapis/python-bigtable/blob/0841824c7b9539df2530370ea128c140eda0f2b2/google/cloud/bigtable_admin_v2/services/bigtable_table_admin/client.py#L1229

Which will end up being pushed down into determine_timeout:
https://github.com/googleapis/python-api-core/blob/ddcc2499fa756f98268eb552edba6f25ddbe0c26/google/api_core/gapic_v1/method.py#L63-L101

Which seems to treat None to mean no deadline and wants Method.DEFAULT to mean the default deadline

Specifically this:

   if specified_timeout is DEFAULT:
        specified_timeout = default_timeout

This effectively means that gapic generated clients will never send a default deadline

@vam-google
Copy link
Contributor

Hi Igor,
The retry, polling and timeotus are in a bad shape in python libraries and we are actively working on cleaning all that up. The default timeout=None is a known issue and is going to be addressed soon in gapic-genertor-python PR, which first needs this big PR be submitted: googleapis/python-api-core#462.

I.e. the issue is much more fundamental bug we are right in the process of fixing it (and also the terminology used is broken)

vam-google added a commit to vam-google/gapic-generator-python that referenced this issue Oct 27, 2022
Also fix LRO for REST transport. This PR makes generated gapics appeciate timeout values from grpc_service_config.json instead of overriding them with None (which means no timeout)

It is basically a direct fix for googleapis#1477.

This PR depends on googleapis/python-api-core#462, and expects `setup.py.j2` templates to be updated after googleapis/python-api-core#462 gets pushed and released with new version.
parthea added a commit that referenced this issue Dec 5, 2022
* fix: Fix timeout default values

Also fix LRO for REST transport. This PR makes generated gapics appeciate timeout values from grpc_service_config.json instead of overriding them with None (which means no timeout)

It is basically a direct fix for #1477.

This PR depends on googleapis/python-api-core#462, and expects `setup.py.j2` templates to be updated after googleapis/python-api-core#462 gets pushed and released with new version.

* rename uri_prefix to path_prefix to match corresponding python-api-core change

* fix unnecessary `gapic_v1.method.DEFAULT` in rest stubs

* fix(deps): require google-api-core >=1.34.0

* fix(deps): require google-api-core >=2.11.0

* revert changes to WORKSPACE

* fix typo

* fix mypy error

* revert local change for debugging

Co-authored-by: Anthonios Partheniou <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants