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 LRO mixin in some APIs #781

Open
2 tasks done
Tracked by #780 ...
coryan opened this issue Jan 22, 2025 · 2 comments · Fixed by #812 · May be fixed by #852
Open
2 tasks done
Tracked by #780 ...

Missing LRO mixin in some APIs #781

coryan opened this issue Jan 22, 2025 · 2 comments · Fixed by #812 · May be fixed by #852
Assignees
Labels
sidekick Issues related to the code generator type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@coryan
Copy link
Contributor

coryan commented Jan 22, 2025

I am not sure if this is a bug in cloud build, or sidekick needs to support services that have LROs but do not have the LRO mixin.

  • google/devtools/cloudbuild/v1
  • google/bigtable/admin/v2

As noted in #780 the code emitted for google/devtools/cloudbuild/v1 does not compile. sidekick is trying to generate the LRO helpers, since there are functions that return google.longrunning.Operation and have the right annotations:

  rpc CreateBuild(CreateBuildRequest) returns (google.longrunning.Operation) {
  ... ... ... snip snip ... ... ...
    option (google.longrunning.operation_info) = {
      response_type: "Build"
      metadata_type: "BuildOperationMetadata"
    };
  }

https://github.com/googleapis/googleapis/blob/e9a4c38a81933108eaa6ac96c7ead31e253c8c64/google/devtools/cloudbuild/v1/cloudbuild.proto#L97-L117

However, the service config does not import the mixin:

type: google.api.Service
config_version: 3
name: cloudbuild.googleapis.com
title: Cloud Build API


apis:
- name: google.devtools.cloudbuild.v1.CloudBuild

https://github.com/googleapis/googleapis/blob/e9a4c38a81933108eaa6ac96c7ead31e253c8c64/google/devtools/cloudbuild/v1/cloudbuild_v1.yaml#L1-L15

Without the mixin, there is no get_operation() function or builder defined. The generated code assumes these exist. One possible "solution" is to disable the LRO helpers if the mixin does not exist, but I am not sure the generated code would be that useful in that case.

Another solution might be to force the mixin when we find LRO-like functions.

@coryan coryan added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. sidekick Issues related to the code generator labels Jan 22, 2025
@coryan coryan changed the title Missing LRO mixin for google/devtools/cloudbuild/v1 Missing LRO mixin in some APIs Jan 22, 2025
@codyoss
Copy link
Member

codyoss commented Jan 22, 2025

Both ways are valid. We need to detect if a service uses LROs and other mixins as well. I know I have seen this sort of thing for IAM as well.

@coryan
Copy link
Contributor Author

coryan commented Jan 26, 2025

We need to automatically insert GetOperation even if it is not in the HTTP rules. See #832 for details.

@coryan coryan linked a pull request Jan 26, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sidekick Issues related to the code generator type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
2 participants