feat: Add code request generator dumper and an ability to run generator from dumped file #765

vam-google · 2021-06-11T09:09:37Z

For debugging purposes.

Usage example:

load("@gapic_generator_java//rules_java_gapic:java_gapic.bzl", "java_generator_request_dump")
java_generator_request_dump(
    name = "compute_small_request_dump",
    srcs = [":compute_small_proto_with_info"],
    transport = "rest",
)

…or from dumped file. For debugging puproses.

snippet-bot · 2021-06-11T09:09:43Z

No region tags are edited in this PR.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

Refresh this comment

codecov · 2021-06-11T09:20:49Z

Codecov Report

Merging #765 (4813464) into master (b7d9399) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #765   +/-   ##
=======================================
  Coverage   91.71%   91.71%           
=======================================
  Files         140      140           
  Lines       14794    14794           
  Branches     1052     1052           
=======================================
  Hits        13568    13568           
  Misses        942      942           
  Partials      284      284

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b7d9399...4813464. Read the comment docs.

miraleung

I came across #767 first, so I left some comments on these files in there.

vam-google · 2021-06-14T23:18:19Z

@miraleung The plan was to review just this one first, merge it and then I could easily merge that into the #767.

vam-google · 2021-06-14T23:28:17Z

BUILD.bazel

@@ -45,6 +45,34 @@ java_binary(
    ],
 )

+java_binary(


@miraleung (from #767): Nit: Can we add a summary comment above these two rules, which also indicate these are debugging tools?

These are not rules, these are java_binaries used by one of the rules only. I clarified the purpose of the binaries in the comments

Thanks for adding the comments. Could we copy-paste these into the Java src files as well? Up to you whether to replace these comments with a pointer to the Java source, or leave these as they are.

Could we also add a usage example? Up to you whether that goes here or in the Java source comments.

src/main/java/com/google/api/generator/Main.java

vam-google · 2021-06-14T23:29:13Z

src/main/java/com/google/api/generator/debug/CodeGeneratorRequestDumper.java

+import com.google.protobuf.compiler.PluginProtos.CodeGeneratorResponse;
+import java.io.IOException;
+
+public class CodeGeneratorRequestDumper {


@miraleung (from #767): If we're using Main in the class names, could we append it here for consistency? Alternatively, remove Main from the other debugger's main class's name.

Those are different things basically. The MainFromFile is literally another variation of the Main class. This, on the other hand, is a very different class, doing different thing. That is why they are named differently. And strictly speaking, this file is really very-very non-main =)

I.e. this class is not really related to Main, while the other one is.

Either way, if you have preferrence how to name these classes, please let me know and I'll rename them. For now, those were the best names I could come up with.

MainStandalone could be something like CodeGeneratorRequestFileToGapicMain?

src/main/java/com/google/api/generator/debug/CodeGeneratorRequestDumper.java

vam-google · 2021-06-14T23:29:47Z

src/main/java/com/google/api/generator/debug/MainFromFile.java

+import java.io.InputStream;
+import java.io.OutputStream;
+
+public class MainFromFile {


@miraleung (from #767): Could we give this a more descriptive name, e.g. GapicToFileMain

Ok, I'll rename. WDYT about naming the Main class to GapicMain also?

About the GapicToFileMain class, I think that would misrepresent what this class does. Its main difference is that it reads/writes from files directly, instead of relying on protoc. I renamed it to MainStandalone (I can add Gapic in name, but in that case we would need to have that prefix in Main class as well, let me know if you want me to do that)

vam-google · 2021-06-14T23:32:07Z

@miraleung I've migrated your comments from the other PR here. Please if anything like this happens, just let me know you prefer a PR to be rebased, so I can rebase it first and then we can continue with the review. Otherwise it generates a lot of unnecessary work for both of us (like migrating these comments).

vam-google · 2021-06-15T01:14:56Z

@miraleung PTAL

vam-google · 2021-06-15T01:15:05Z

@miraleung PTAL

miraleung

LGTM with comments addressed.

miraleung · 2021-06-15T18:39:36Z

src/main/java/com/google/api/generator/debug/CodeGeneratorRequestDumper.java

+import com.google.protobuf.compiler.PluginProtos.CodeGeneratorResponse;
+import java.io.IOException;
+
+public class CodeGeneratorRequestDumper {


MainStandalone could be something like CodeGeneratorRequestFileToGapicMain?

miraleung · 2021-06-15T18:41:35Z

BUILD.bazel

@@ -45,6 +45,34 @@ java_binary(
    ],
 )

+java_binary(


Thanks for adding the comments. Could we copy-paste these into the Java src files as well? Up to you whether to replace these comments with a pointer to the Java source, or leave these as they are.

Could we also add a usage example? Up to you whether that goes here or in the Java source comments.

🤖 I have created a release *beep* *boop* --- ### Updating meta-information for bleeding-edge SNAPSHOT release. --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

….6.0 (#765) [![WhiteSource Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [com.google.auth:google-auth-library-bom](https://togithub.com/googleapis/google-auth-library-java) | `1.5.3` -> `1.6.0` | [![age](https://badges.renovateapi.com/packages/maven/com.google.auth:google-auth-library-bom/1.6.0/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/maven/com.google.auth:google-auth-library-bom/1.6.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/maven/com.google.auth:google-auth-library-bom/1.6.0/compatibility-slim/1.5.3)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/maven/com.google.auth:google-auth-library-bom/1.6.0/confidence-slim/1.5.3)](https://docs.renovatebot.com/merge-confidence/) | --- ### Configuration 📅 **Schedule**: At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/java-core).

vam-google added 4 commits June 1, 2021 23:16

feat: implement DIREGAPIC

8983e23

Merge branch 'diregapic'

83e004f

feat: Add code request generator dumper and an ability to run generat…

a978fff

…or from dumped file. For debugging puproses.

reformat code

310737e

vam-google requested review from miraleung and a team as code owners June 11, 2021 09:09

google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Jun 11, 2021

miraleung suggested changes Jun 14, 2021

View reviewed changes

vam-google commented Jun 14, 2021

View reviewed changes

vam-google added a commit to vam-google/gapic-generator-java that referenced this pull request Jun 14, 2021

remove the googleapis#765 parts

65b88e2

vam-google mentioned this pull request Jun 14, 2021

feat: Add DIREGAPIC-specific pagination #767

Merged

address PR feedback

9387e64

vam-google requested a review from miraleung June 15, 2021 00:08

miraleung approved these changes Jun 15, 2021

View reviewed changes

vam-google added 2 commits June 15, 2021 22:04

Merge remote-tracking branch 'upstream/master'

1c4453c

address PR feedback

4813464

vam-google merged commit 4f187b1 into googleapis:master Jun 16, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add code request generator dumper and an ability to run generator from dumped file #765

feat: Add code request generator dumper and an ability to run generator from dumped file #765

vam-google commented Jun 11, 2021

snippet-bot bot commented Jun 11, 2021

codecov bot commented Jun 11, 2021 •

edited

Loading

miraleung left a comment

vam-google commented Jun 14, 2021

vam-google Jun 14, 2021

vam-google Jun 14, 2021

miraleung Jun 15, 2021

vam-google Jun 16, 2021

vam-google Jun 14, 2021

vam-google Jun 14, 2021 •

edited

Loading

miraleung Jun 15, 2021

vam-google Jun 16, 2021

vam-google Jun 14, 2021

vam-google Jun 14, 2021 •

edited

Loading

vam-google commented Jun 14, 2021

vam-google commented Jun 15, 2021

vam-google commented Jun 15, 2021

miraleung left a comment

miraleung Jun 15, 2021

miraleung Jun 15, 2021

feat: Add code request generator dumper and an ability to run generator from dumped file #765

feat: Add code request generator dumper and an ability to run generator from dumped file #765

Conversation

vam-google commented Jun 11, 2021

snippet-bot bot commented Jun 11, 2021

No region tags are edited in this PR.

codecov bot commented Jun 11, 2021 • edited Loading

Codecov Report

miraleung left a comment

Choose a reason for hiding this comment

vam-google commented Jun 14, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vam-google Jun 14, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vam-google Jun 14, 2021 • edited Loading

Choose a reason for hiding this comment

vam-google commented Jun 14, 2021

vam-google commented Jun 15, 2021

vam-google commented Jun 15, 2021

miraleung left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jun 11, 2021 •

edited

Loading

vam-google Jun 14, 2021 •

edited

Loading

vam-google Jun 14, 2021 •

edited

Loading