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

feat: add support for long-running operations with rest transport. #1094

Merged
merged 8 commits into from Nov 25, 2021

Conversation

kbandes
Copy link
Contributor

@kbandes kbandes commented Nov 19, 2021

This PR adds support for long-running operations in the rest transport. Some noteworthy points:

  • the python gapic generator now reads the service yaml file for an api, using a new service_yaml parameter in py_gapic_library targets. The content of this file is loaded into a google.api.Service proto and is available in the schema as the API.service_yaml_config property. This can, in the future, support other features that rely on information in this file. Note that the java, golang, php, and nodejs targets already have this parameter.

  • In addition to unit tests, this change has been tested against a live server using the cloud translate api.
    Note that this change is dependent upon feat: add operations rest client to support long-running operations. python-api-core#311.

@kbandes kbandes requested review from a team as code owners November 19, 2021 00:46
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Nov 19, 2021
Copy link
Contributor

@parthea parthea left a comment

Choose a reason for hiding this comment

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

Added minor observations

kbandes and others added 2 commits November 20, 2021 09:02
…ce/transports/rest.py.j2

Co-authored-by: Anthonios Partheniou <partheniou@google.com>
…ce/transports/rest.py.j2

Co-authored-by: Anthonios Partheniou <partheniou@google.com>
gapic/schema/api.py Outdated Show resolved Hide resolved
gapic/utils/options.py Outdated Show resolved Hide resolved
@software-dov
Copy link
Contributor

Looks good, just two minor nits that shouldn't block the merge. Feel free to integrate them or ignore them.

@kbandes
Copy link
Contributor Author

kbandes commented Nov 22, 2021

I'll take a look. In some cases the only way I could resolve conflicts between black and flake8 was to remove trailing commas, but that was a different repo. I'll try it.

@kbandes
Copy link
Contributor Author

kbandes commented Nov 23, 2021

I need to make additional changes:

  1. OperationsRestClient has been renamed to AbstractOperationsClient.
  2. The generated tests for rest now fail, I think because the Operations rest transport is not being mocked.

@kbandes kbandes merged commit e89fd23 into googleapis:master Nov 25, 2021
@kbandes kbandes deleted the kbandes-lro branch November 25, 2021 02:19
gcf-merge-on-green bot pushed a commit that referenced this pull request Dec 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants