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

Generator incorrectly sets the composeTransport sample's region tag #1272

Closed
lqiu96 opened this issue Jan 25, 2023 · 3 comments · Fixed by #3293
Closed

Generator incorrectly sets the composeTransport sample's region tag #1272

lqiu96 opened this issue Jan 25, 2023 · 3 comments · Fixed by #3293
Assignees
Labels
priority: p3 Desirable enhancement or fix. May not be included in next release. samples Issues that are directly related to samples. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@lqiu96
Copy link
Contributor

lqiu96 commented Jan 25, 2023

Context:
The SyncCreateSetCredentialsProvider sample already exists: https://github.com/googleapis/google-cloud-java/blob/56c9a8a29c1d04baed958cc028918eea5995aa17/java-tpu/samples/snippets/generated/com/google/cloud/tpu/v2/tpu/create/SyncCreateSetCredentialsProvider.java

and it properly showcases setting the credentials. The SyncCreateSetCredentialsProvider1 sample should be named something that reflects that it is using httpjson.

This generator's logic seems to set the wrong region tag name:
https://github.com/googleapis/gapic-generator-java/blob/8ef747b244c9bed128252cb6913d954ae7270251/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/samplecode/ServiceClientHeaderSampleComposer.java#L345

The logic may have been mistakenly copied over. This is creating samples with SyncCreateSetCredentialsProvider1 as the filename instead of something like ComposeTranport or UseHttpJsonTransport.

https://github.com/googleapis/google-cloud-java/blob/56c9a8a29c1d04baed958cc028918eea5995aa17/java-tpu/samples/snippets/generated/com/google/cloud/tpu/v2/tpu/create/SyncCreateSetCredentialsProvider1.java#L23

@lqiu96 lqiu96 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 Jan 25, 2023
@product-auto-label product-auto-label bot added the samples Issues that are directly related to samples. label Jan 25, 2023
@suztomo
Copy link
Member

suztomo commented Apr 5, 2023

@lqiu96 Is the screenshot below capturing the incorrectness you found?

image

@suztomo suztomo added priority: p3 Desirable enhancement or fix. May not be included in next release. and removed priority: p2 Moderately-important priority. Fix may not be included in next release. labels Apr 5, 2023
@lqiu96
Copy link
Contributor Author

lqiu96 commented Apr 5, 2023

I'll update the title/body to reflect the entire scope for more clarity.

Thanks for the screenshot! The screenshot shows the invalid region tag. The other issue is that the file name/ class is set incorrectly (which I'm fairly certain is due to the region tag name).

Context:
The SyncCreateSetCredentialsProvider sample already exists: https://github.com/googleapis/google-cloud-java/blob/56c9a8a29c1d04baed958cc028918eea5995aa17/java-tpu/samples/snippets/generated/com/google/cloud/tpu/v2/tpu/create/SyncCreateSetCredentialsProvider.java

and it properly tests setting the credentials. The SyncCreateSetCredentialsProvider1 sample should be named something that reflects that it is using httpjson.

@lqiu96 lqiu96 changed the title Update composeTransport sample to the correct region tag Generator incorrectly sets the composeTransport sample's region tag Apr 5, 2023
@suztomo
Copy link
Member

suztomo commented Apr 11, 2024

As of today, the generated sample still has the name SyncCreateSetCredentialsProvider1:

suztomo@suztomo2:~/google-cloud-java$ grep -ir SyncCreateSetCredentialsProvider java-tpu
java-tpu/samples/snippets/generated/com/google/cloud/tpu/v2alpha1/tpu/create/SyncCreateSetCredentialsProvider.java:public class SyncCreateSetCredentialsProvider {
java-tpu/samples/snippets/generated/com/google/cloud/tpu/v2alpha1/tpu/create/SyncCreateSetCredentialsProvider.java:    syncCreateSetCredentialsProvider();
java-tpu/samples/snippets/generated/com/google/cloud/tpu/v2alpha1/tpu/create/SyncCreateSetCredentialsProvider.java:  public static void syncCreateSetCredentialsProvider() throws Exception {
java-tpu/samples/snippets/generated/com/google/cloud/tpu/v1/tpu/create/SyncCreateSetCredentialsProvider.java:public class SyncCreateSetCredentialsProvider {
java-tpu/samples/snippets/generated/com/google/cloud/tpu/v1/tpu/create/SyncCreateSetCredentialsProvider.java:    syncCreateSetCredentialsProvider();
java-tpu/samples/snippets/generated/com/google/cloud/tpu/v1/tpu/create/SyncCreateSetCredentialsProvider.java:  public static void syncCreateSetCredentialsProvider() throws Exception {
java-tpu/samples/snippets/generated/com/google/cloud/tpu/v2/tpu/create/SyncCreateSetCredentialsProvider.java:public class SyncCreateSetCredentialsProvider {
java-tpu/samples/snippets/generated/com/google/cloud/tpu/v2/tpu/create/SyncCreateSetCredentialsProvider.java:    syncCreateSetCredentialsProvider();
java-tpu/samples/snippets/generated/com/google/cloud/tpu/v2/tpu/create/SyncCreateSetCredentialsProvider.java:  public static void syncCreateSetCredentialsProvider() throws Exception {
java-tpu/samples/snippets/generated/com/google/cloud/tpu/v2/tpu/create/SyncCreateSetCredentialsProvider1.java:public class SyncCreateSetCredentialsProvider1 {
java-tpu/samples/snippets/generated/com/google/cloud/tpu/v2/tpu/create/SyncCreateSetCredentialsProvider1.java:    syncCreateSetCredentialsProvider1();
java-tpu/samples/snippets/generated/com/google/cloud/tpu/v2/tpu/create/SyncCreateSetCredentialsProvider1.java:  public static void syncCreateSetCredentialsProvider1() throws Exception {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p3 Desirable enhancement or fix. May not be included in next release. samples Issues that are directly related to samples. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
4 participants