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

Turn on snippetgen by default #1043

Closed
busunkim96 opened this issue Oct 27, 2021 · 5 comments
Closed

Turn on snippetgen by default #1043

busunkim96 opened this issue Oct 27, 2021 · 5 comments
Assignees
Labels
type: process A process-related concern. May include testing, release, or the like.

Comments

@busunkim96
Copy link
Contributor

CC @dizcology

There's a small chance snippetgen falls apart on a real API, so I plan to take the following steps to avoid blocking other updates:

@busunkim96 busunkim96 added the type: process A process-related concern. May include testing, release, or the like. label Oct 27, 2021
@busunkim96 busunkim96 self-assigned this Oct 27, 2021
@busunkim96
Copy link
Contributor Author

There appears to be a bug for client streaming so I wasn't able to enable this globally:

If it takes a while to fix I will back out #1044 so it doesn't block other changes.

    sample["request"] = v.validate_and_transform_request(
  File "/usr/local/google/home/busunkim/.cache/bazel/_bazel_busunkim/5f7ad274936c83fa5125630e373edd54/sandbox/linux-sandbox/1362/execroot/com_google_googleapis/bazel-out/host/bin/external/gapic_generator_python/gapic_plugin.runfiles/gapic_generator_python/gapic/samplegen/samplegen.py", line 560, in validate_and_transform_request
    raise types.InvalidRequestSetup(
gapic.samplegen_utils.types.InvalidRequestSetup: Too many base parameters for client side streaming form
--python_gapic_out: protoc-gen-python_gapic: Plugin failed with status code 1.
Target //google/firestore/v1:firestore-v1-py failed to build

busunkim96 added a commit that referenced this issue Nov 8, 2021
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.
@busunkim96
Copy link
Contributor Author

busunkim96 commented Nov 10, 2021

Aand I've found at least one other bug - it was not caught by the goldens but was discovered when I tried to use #1077 as the generator version in the WORKSPACE file. I think it is related to #1012.

I'm planning on manually generating locally with 0.56.1 with every API to shake out remaining bugs blocking turning on snippetgen by default.

@busunkim96
Copy link
Contributor Author

#1012 started to use the resource path logic which wasn't written to handle nested sub-fields. The generation was failing as matching resource paths were not found for resource paths that are not on the base request.

@busunkim96
Copy link
Contributor Author

I've opened #1144 to track re-adding f-string support for resource paths (it requires addressing the bug mentioned in the last comment).

The resource path logic appears to have been the last blocking issue. I ran the internal googleapis presubmit with #1134 and all the APIs appear to be generate successfully. 😃

@parthea
Copy link
Contributor

parthea commented Apr 1, 2022

@busunkim96 Please can you check if there are any outstanding items? If not could we close this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: process A process-related concern. May include testing, release, or the like.
Projects
None yet
Development

No branches or pull requests

2 participants