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

Noisy changes appear in autogenerated snippets #1457

Closed
parthea opened this issue Sep 21, 2022 · 1 comment · Fixed by #1463
Closed

Noisy changes appear in autogenerated snippets #1457

parthea opened this issue Sep 21, 2022 · 1 comment · Fixed by #1463
Assignees
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@parthea
Copy link
Contributor

parthea commented Sep 21, 2022

There is a lot of noise in the generated output, in particular the snippetgen json files, which started about 2 weeks ago where the filenames in the snippetgen json are changing. The issue only appears in clients where we've enabled REST transport. For example in https://github.com/googleapis/python-dataflow, we enabled REST transport in googleapis/python-dataflow-client#139 and the PRs afterwards contained noisy changes

https://github.com/googleapis/python-dataflow-client/pull/141/files
https://github.com/googleapis/python-dataflow-client/pull/143/files

See the screenshot below where the value for file is changed from dataflow_v1beta3_generated_jobs_v1_beta3_aggregated_list_jobs_sync_26f07383.py to dataflow_v1beta3_generated_jobs_v1_beta3_aggregated_list_jobs_sync_0d901b38.py
image

@parthea parthea added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p2 Moderately-important priority. Fix may not be included in next release. labels Sep 21, 2022
@dizcology
Copy link
Collaborator

dizcology commented Sep 30, 2022

The issue is caused by samplegen not handling the REST transport type properly, and buckets that with TRANSPORT_GRPC_ASYNC: https://github.com/googleapis/gapic-generator-python/blob/main/gapic/samplegen/samplegen.py#L1009. This causes samplegen to later on see two samples (REST and GRPC_SYNC) with the same spec, and is forced to use a hash in the filename.

I think the generated snippets for REST should be different from those for GRPC_SYNC (should specify the REST transport during client instantiation). At this point it appears that samplegen does not correctly generate REST samples by comparing "sync" samples with hash. Because of this, we should disable snippet generation for REST until samplegen supports that variant.

I will create a PR for this soon. @parthea let me know if you have concerns about this. (Note that this means there will be new PRs that remove these duplicate snippets.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants