feat: REST GAPIC (REGAPIC) Support for Java #3275

vam-google · 2020-09-10T00:05:02Z

This PR depends on googleapis/gax-java#1177 and provides MVP for generating rest gapic clients. It was tested on real GCE API and was able to produce a valid client.

codecov · 2020-09-10T00:20:42Z

Codecov Report

Merging #3275 into master will decrease coverage by 0.00%.
The diff coverage is 84.68%.

@@             Coverage Diff              @@
##             master    #3275      +/-   ##
============================================
- Coverage     87.12%   87.12%   -0.01%     
- Complexity     6081     6105      +24     
============================================
  Files           494      495       +1     
  Lines         24066    24143      +77     
  Branches       2615     2627      +12     
============================================
+ Hits          20967    21034      +67     
- Misses         2236     2247      +11     
+ Partials        863      862       -1

Impacted Files	Coverage Δ	Complexity Δ
...oogle/api/codegen/gapic/GapicGeneratorFactory.java	`88.95% <ø> (ø)`	`12.00 <0.00> (ø)`
...codegen/viewmodel/testing/ClientTestClassView.java	`88.88% <ø> (ø)`	`4.00 <0.00> (ø)`
...le/api/codegen/viewmodel/testing/TestCaseView.java	`100.00% <ø> (ø)`	`2.00 <0.00> (ø)`
...om/google/api/codegen/gapic/GapicGeneratorApp.java	`53.84% <50.00%> (-0.26%)`	`6.00 <0.00> (ø)`
...degen/transformer/java/JavaSurfaceTransformer.java	`97.15% <60.00%> (-0.91%)`	`73.00 <0.00> (+2.00)`	⬇️
...ain/java/com/google/api/codegen/GeneratorMain.java	`31.86% <72.72%> (+1.07%)`	`3.00 <0.00> (ø)`
...pi/codegen/transformer/ApiCallableTransformer.java	`96.40% <93.33%> (+0.10%)`	`47.00 <5.00> (+10.00)`
...ogle/api/codegen/config/GapicInterfaceContext.java	`95.83% <100.00%> (+1.61%)`	`49.00 <3.00> (+3.00)`
.../google/api/codegen/config/GapicProductConfig.java	`82.05% <100.00%> (-0.04%)`	`104.00 <1.00> (ø)`
...e/api/codegen/transformer/TestCaseTransformer.java	`98.59% <100.00%> (+<0.01%)`	`54.00 <0.00> (ø)`
... and 12 more

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 ae9338a...9219acf. Read the comment docs.

miraleung · 2020-09-12T00:24:46Z

rules_gapic/gapic.bzl

@@ -40,11 +40,13 @@ def _gapic_srcjar_impl(ctx):
        _set_args(attr.service_yaml, "--service_yaml=", arguments, inputs)
        _set_args(attr.package_yaml2, "--package_yaml2=", arguments, inputs)
        _set_args(attr.grpc_service_config, "--grpc_service_config=", arguments, inputs)
+        _set_args(attr.transport_protocol, "--transport_protocol=", arguments)


Could we please document the valid values of transport_protocol somewhere?

I updated the code to match it the main regapic spec. Those values are documented there (and this argument was changed from transport_protocol to just transport). The values are grpc, rest, grpc+rest. This implementation does not support grpc+rest because it is not needed for GCE (the main thing we need now), and investing time end effort into supporting the proper grpc+rest in monolith is probably not worth it.

Added the list of possible values in the comment for the corresponding command line argumetn in GeneratorMain.

miraleung · 2020-09-12T00:25:36Z

rules_gapic/java/java_gapic.bzl

@@ -175,6 +176,17 @@ def java_gapic_library(
        "@javax_annotation_javax_annotation_api//jar",
    ]

+    if transport_protocol == "http":


It looks like either HTTP or gRPC will be generated, not both. Would it make sense to make transport_protocol a boolean flag that indicates whether it's HTTP or not?

I changed transport_protocol to transport. The name and the possible values for the argument now match the general REGAPIC spec.

miraleung · 2020-09-12T00:29:43Z

src/main/java/com/google/api/codegen/transformer/java/JavaGapicSurfaceTransformer.java

@@ -42,9 +43,11 @@
  private static final String SETTINGS_TEMPLATE_FILENAME = "java/settings.snip";
  private static final String STUB_SETTINGS_TEMPLATE_FILENAME = "java/stub_settings.snip";
  private static final String STUB_INTERFACE_TEMPLATE_FILENAME = "java/stub_interface.snip";
-  private static final String GRPC_STUB_TEMPLATE_FILENAME = "java/grpc_stub.snip";
+  private static final String STUB_TEMPLATE_FILENAME = "java/stub.snip";


Since only one of HTTP or gRPC will be generated, would it make sense to split the template into grpc_stub.snip and http_stub.snip? As a reader, I would prefer duplicate code to parsing the diffs (which is less clear without doing a deep dive).

Ditto for test.snip.

GRPC and REST stubs share too much (like 90-80% of the file is the same). Keeping them separate makes it much easier for rest and grpc stubs to drift from each other leading to troubles supporting it.

I tried to make the .ship files match the philosophical view on the two different transports implementations - they are essentially the same thing with some transport-specific tweaks. Duplicating the .snip files for each transport views them differently - two transports are two separate independent things.

…m -> putQueryParam, toPathParam -> putPathParam)

vam-google · 2020-09-16T07:08:50Z

@miraleung PTAL

vchudnov-g

Looking good!

(You submitted a commit while I was reviewing, so I'll do another pass after you respond)

vchudnov-g · 2020-09-22T21:03:09Z

rules_gapic/java/java_gapic.bzl

@@ -175,6 +176,17 @@ def java_gapic_library(
        "@javax_annotation_javax_annotation_api//jar",
    ]

+    if transport == "rest":


How about doing these additively?

all_transports=transport.split("+") if transport else ["grpc","rest"] # [2] # [1] if "rest" in all_transports: # add rest deps if "grpc" in all_transports: # add grpc deps

If you don't want to support rest+grpc for now, in #[1] you can add
all_transports = all_transports[0:1] # TODO: remove when we generate multi-transport GAPIC

(Maybe you can factor out #[1] and #[2] into a helper function, since this conditional appears another time below.)

I'm trying to keep it backward-compatible with the older version of the rules, when no transport argument meant grpc. If the implementation of gapic generator does not support rest+grpc I don't think it makes much sense to support that in the rules. The current implementation is both backward compatible and the simplest I could come up with. Doing removal would mean that rest+grpc and grpc+rest "rest" only which is confusing and more complicated behavior.

Note, it is very unlikely that grpc+rest will ever be implemented in monolith. This is the main reason why the rest of this implementation avoids going into complications of grpc+rest support (including bazel rules or command line args parsing).

+1 to backwards-compatibility.

The code I suggest above is backwards-compatible (if you include the line I suggest for #[1]). It does everything your existing code does, but makes it clear that the intent is to support both protocols eventually. The line I suggest for #[1] above makes it clear that dual protocols are not supported yet (and as you say, may never be in the monolith).

I strongly urge you to consider this because it's only a hair less simple than what you have now and it makes the project intention very clear by putting in the structure that we would have in a full implementation. In my experience, this is a good principle because of that clarity and because it diminishes the technical debt should, say, the monolith be in use longer than we're planning now.

(I'm not going to block on this, but do consider it)

Note, all this is essentially a throw away code, because the final destination of it will be in java microgenerator. Please lets not try to polish throwaway code or add features which most likely will never be fully implemented here (like the http+grpc option).

It's a very simple change that increases clarity, IMO. My preference would be to add it. But as I said, I'm not going to block on this.

vchudnov-g · 2020-09-22T21:14:22Z

rules_gapic/java/java_gapic.bzl

        "@junit_junit//jar",
    ]

+    if transport == "rest":


See comment above.

Please lets keep it simple, not trying to support in the rules something which is not supported in the generator itself.

I fully agree with YAGNI, but I think the structure I propose is only a hair less simple and makes this part just work, increasing clarity.

vchudnov-g · 2020-09-22T21:17:25Z

rules_gapic/java/java_gapic_pkg.bzl

@@ -206,9 +207,14 @@ def java_gapic_assembly_gradle_pkg(
        grpc_target_dep = ["%s" % grpc_target]

    if client_deps:
+        if transport == "rest":


I would use a similar construct to above to get all the transports, and then be explicit about the special casing if we're not yet generating multi-transport GAPICs.

This rest, grpc, rest+grpc, grpc+rest thing gets propagated in many places (like the above and the bottom comments). By doing it this way I was trying to keep it as simple as possible and not trying to implement in the rules something which is not there in the generator.

I was aiming to have the interface (the rules) have the desired surface, and then provide stubs for what is not implemented (ie dual transport fails for now)

rules_gapic/java/java_gapic_pkg.bzl

vchudnov-g · 2020-09-22T21:22:24Z

src/main/java/com/google/api/codegen/GeneratorMain.java

+  private static final Option TRANSPORT =
+      Option.builder()
+          .longOpt("transport") // Possible values: grpc, rest or grpc+rest (not supported yet).
+          .desc("Transport used by the generated clients.")


I suggest removing the comment above and including the info in the description:
"List of transports the GAPIC should support. Valid transport names ('grpc' or 'rest') are separated by '+'. Default is 'grpc+rest'. NOTE: for now, GAPICs support only the first transport in the list."

Done, but keeping grpc as default for backward compatibility.

src/main/java/com/google/api/codegen/viewmodel/StaticLangRpcStubView.java

vchudnov-g · 2020-09-22T23:34:36Z

src/main/resources/com/google/api/codegen/java/stub.snip

@@ -58,6 +62,79 @@
          .build();
 @end

+@private httpMethodDescriptor(methodDescriptor)


nit: I'd lean towards s/http/rest/ but if you think this is better for consistency with existing internal code, that's fine.

Yes, this is for consistency and compatibility with discogapic. I can't make big changes here untill discogapic is removed.

src/test/java/com/google/api/codegen/gapic/GapicCodeGeneratorTest.java

vchudnov-g · 2020-09-22T23:44:24Z

src/test/java/com/google/api/codegen/gapic/ProtocGapicPluginGeneratorTest.java

@@ -53,7 +53,7 @@ public void testGenerator() {
                model.getFiles().stream().map(ProtoFile::getProto).collect(Collectors.toList()))
            // Only the file to generate a client for (don't generate dependencies)
            .addFileToGenerate("multiple_services.proto")
-            .setParameter("language=java")
+            .setParameter("language=java,transport=grpc")


can we also test for transport=rest?

We technically do, on line 76. But seriously speaking, these tests are useless, because gapic-generator is never executed as plugin in production, so they are testing dead logic.

But we are providing the functionality in the generator, even if we ourselves don't use it, so it should be tested. Line 76 tests the failing case.

All that said, it's only worth doing if it exercises additional code. I think this will test that the high-level transport selection doesn't error out, right? So it might be worth including. Can you simply copy this existing test case and change the transport (and maybe check for the presence of a REST-specific string, if that's simple).

vchudnov-g · 2020-09-22T23:53:37Z

src/test/java/com/google/api/codegen/testsrc/protoannotations/library.proto

@@ -86,7 +86,7 @@ service LibraryService {

  // Deletes a shelf.
  rpc DeleteShelf(DeleteShelfRequest) returns (google.protobuf.Empty) {
-    option (google.api.http) = { delete: "/v1/{name=bookShelves/*}" };
+    option (google.api.http) = { delete: "/v1/bookShelves/{name}" };


So we're supporting both /{name=bookShelves/*} and /bookShelves/{name}? It looks from https://google.aip.dev/127 that only the former is that standard. What am I overlooking?

Both works, even if they are not listed in AIP127, so testing both.

But if it's not in the AIP, we want to NOT support it. We don't want people annotating their gRPC APIs incorrectly, so we need to fail when they do. If the unapproved pattern is something that the converter is outputting into the synthetic protos, let's fix that. At the very least, could you file a bug in the converter repo and one in this repo, and add a TODO here to restrict to the supported pattern?

This would require non-trivial unnecessary conversions in the converter which I really would like to avoid (one more thing to break potentially). This works out of the box in Java and PHP and I'm pretty sure it will work in other langauges as well. If anything, I believe it is AIP description itself is incomplete, not the pattern used here is wrong. Note, that AIP does not really attempt to provide a full spec of how the template strings may look like.

Lets discuss it in person, I feel strong about not making the unnecessary conversion here (we are converting something simple to something more complicted, in a situation when it really looks unnecessary, to follow AIP which is not attempting to rigorously define how these template strings should look like).

Btw, Looks like the annotation itself officially supports the plain syntax: https://github.com/googleapis/googleapis/blob/master/google/api/http.proto#L96

Short version: OK, but add a note.

Long version:

In our discussion today we said that the HTTP annotation is independent of resource name formatting. It just says how to substitute the fields into an HTTP pattern, not how that pattern should look like,

I think that's wrong, though: from the AIP:

URIs must use the {foo=bar/*} syntax to represent a variable that should be populated in the request proto. When extracting a resource name, the variable must include the entire resource name, not just the ID component.

That said: it's OK for the monolith to accept this input for now. I don't think microgenerators should. I would put a TODO here noting that this is non-standard, even though we won't fix it in the monolith

(And the converter emits protos in the correct format, right?)

To be clear, this isn't blocking this PR.

On the generation side, I checked and you're right, this pattern is intended to be permissible. The relevant AIP here is AIP 4231 (Could you add a reference to that in the converter design doc, please?). I'll work with Luke to make the wording on AIP 127 less misleading (to me).

vam-google · 2020-09-23T07:51:37Z

@vchudnov-g @miraleung PTAL

miraleung · 2020-09-25T00:06:20Z

rules_gapic/java/java_gapic.bzl

@@ -175,6 +176,17 @@ def java_gapic_library(
        "@javax_annotation_javax_annotation_api//jar",
    ]

+    if transport == "rest":


+1 to backwards-compatibility.

miraleung · 2020-09-25T00:08:04Z

src/main/java/com/google/api/codegen/gapic/GapicGeneratorApp.java

+      ToolOptions.createOption(
+          String.class,
+          "transport",
+          "List of transports to use ('rest' or 'grpc') separated by '+'. NOTE: For now"


Should this just be "Transport to use" since it's not a List option?

But you can list more than one, separated by "+". It's a string representation of a +-separated list.

Again, change the NOTE to be accurate: for now, we only support a single transport.

miraleung · 2020-09-25T00:08:55Z

src/main/java/com/google/api/codegen/transformer/java/JavaGapicSurfaceTransformer.java

@@ -42,9 +43,11 @@
  private static final String SETTINGS_TEMPLATE_FILENAME = "java/settings.snip";
  private static final String STUB_SETTINGS_TEMPLATE_FILENAME = "java/stub_settings.snip";
  private static final String STUB_INTERFACE_TEMPLATE_FILENAME = "java/stub_interface.snip";
-  private static final String GRPC_STUB_TEMPLATE_FILENAME = "java/grpc_stub.snip";
+  private static final String STUB_TEMPLATE_FILENAME = "java/stub.snip";


vchudnov-g

I have a concern about AIP127; help me understand that. And then the help text in GeneratorMain I doesn't quite match the current implementation.

vchudnov-g · 2020-09-28T18:48:38Z

rules_gapic/java/java_gapic.bzl

@@ -175,6 +176,17 @@ def java_gapic_library(
        "@javax_annotation_javax_annotation_api//jar",
    ]

+    if transport == "rest":


The code I suggest above is backwards-compatible (if you include the line I suggest for #[1]). It does everything your existing code does, but makes it clear that the intent is to support both protocols eventually. The line I suggest for #[1] above makes it clear that dual protocols are not supported yet (and as you say, may never be in the monolith).

I strongly urge you to consider this because it's only a hair less simple than what you have now and it makes the project intention very clear by putting in the structure that we would have in a full implementation. In my experience, this is a good principle because of that clarity and because it diminishes the technical debt should, say, the monolith be in use longer than we're planning now.

(I'm not going to block on this, but do consider it)

vchudnov-g · 2020-09-28T18:50:46Z

rules_gapic/java/java_gapic.bzl

        "@junit_junit//jar",
    ]

+    if transport == "rest":


I fully agree with YAGNI, but I think the structure I propose is only a hair less simple and makes this part just work, increasing clarity.

vchudnov-g · 2020-09-28T18:53:20Z

rules_gapic/java/java_gapic_pkg.bzl

@@ -206,9 +207,14 @@ def java_gapic_assembly_gradle_pkg(
        grpc_target_dep = ["%s" % grpc_target]

    if client_deps:
+        if transport == "rest":


I was aiming to have the interface (the rules) have the desired surface, and then provide stubs for what is not implemented (ie dual transport fails for now)

vchudnov-g · 2020-09-28T18:54:34Z

src/main/java/com/google/api/codegen/gapic/GapicGeneratorApp.java

+      ToolOptions.createOption(
+          String.class,
+          "transport",
+          "List of transports to use ('rest' or 'grpc') separated by '+'. NOTE: For now"


But you can list more than one, separated by "+". It's a string representation of a +-separated list.

vchudnov-g · 2020-09-28T18:57:54Z

src/main/java/com/google/api/codegen/gapic/GapicGeneratorApp.java

+    String transport = options.get(TRANSPORT).toLowerCase();
+
+    TransportProtocol tp;
+    if (transport.equals("grpc")) {


OK. My own preference, as I said above, is to have the right outer structure and just stubs for the unimplemented parts (ie multi-transport). It's just as simple and clearer. But I feel more strongly about it in the outermost surface (Bazel) than in the generator.

vchudnov-g · 2020-09-28T19:04:05Z

src/main/java/com/google/api/codegen/transformer/java/JavaSurfaceTransformer.java

-        typeTable.saveNicknameFor("com.google.api.gax.httpjson.ApiMessageHttpRequestFormatter");
-        typeTable.saveNicknameFor("com.google.api.gax.httpjson.ApiMessageHttpResponseParser");
+        String configSchemaVersion = context.getProductConfig().getConfigSchemaVersion();
+        if (configSchemaVersion != null && configSchemaVersion.startsWith("1.")) {


Yeah. The comment is very helpful! (As in the other file where you also added it.) Thanks!

vchudnov-g · 2020-09-28T19:06:42Z

src/main/resources/com/google/api/codegen/java/stub.snip

@@ -58,6 +62,79 @@
          .build();
 @end

+@private httpMethodDescriptor(methodDescriptor)


vchudnov-g · 2020-09-28T19:19:13Z

src/test/java/com/google/api/codegen/gapic/ProtocGapicPluginGeneratorTest.java

@@ -53,7 +53,7 @@ public void testGenerator() {
                model.getFiles().stream().map(ProtoFile::getProto).collect(Collectors.toList()))
            // Only the file to generate a client for (don't generate dependencies)
            .addFileToGenerate("multiple_services.proto")
-            .setParameter("language=java")
+            .setParameter("language=java,transport=grpc")


But we are providing the functionality in the generator, even if we ourselves don't use it, so it should be tested. Line 76 tests the failing case.

All that said, it's only worth doing if it exercises additional code. I think this will test that the high-level transport selection doesn't error out, right? So it might be worth including. Can you simply copy this existing test case and change the transport (and maybe check for the presence of a REST-specific string, if that's simple).

vchudnov-g · 2020-09-28T19:25:31Z

src/test/java/com/google/api/codegen/testsrc/protoannotations/library.proto

@@ -86,7 +86,7 @@ service LibraryService {

  // Deletes a shelf.
  rpc DeleteShelf(DeleteShelfRequest) returns (google.protobuf.Empty) {
-    option (google.api.http) = { delete: "/v1/{name=bookShelves/*}" };
+    option (google.api.http) = { delete: "/v1/bookShelves/{name}" };


But if it's not in the AIP, we want to NOT support it. We don't want people annotating their gRPC APIs incorrectly, so we need to fail when they do. If the unapproved pattern is something that the converter is outputting into the synthetic protos, let's fix that. At the very least, could you file a bug in the converter repo and one in this repo, and add a TODO here to restrict to the supported pattern?

vchudnov-g · 2020-09-28T19:32:53Z

src/main/java/com/google/api/codegen/GeneratorMain.java

+          .longOpt("transport")
+          .desc(
+              "List of transports to support. Valid transport names ('grpc' or 'rest') are"
+                  + " separated by '+'. Default is 'grpc'. NOTE: for now, GAPICs support only"


This comment as phrased makes sense with the code changes I suggested in the last pass, where (a) the default list is grpc+rest and (b) because we're not yet implementing dual transport, we then look at only the first element in the list as per my #[1] comment: all_transports = all_transports[0:1]

Please update the comment if you're not handling the list option: you're not handling the "+" separated list unless you make changes like the ones I suggested,.

vchudnov-g

Please address the doc comments and the comments I pinged you on.

(BTW: I think if you reply to my review with a review of your own, then it will show up in my dashboard. I think that's why I didn't see this earlier.)

vchudnov-g · 2020-10-06T21:31:26Z

rules_gapic/java/java_gapic.bzl

@@ -175,6 +176,17 @@ def java_gapic_library(
        "@javax_annotation_javax_annotation_api//jar",
    ]

+    if transport == "rest":


It's a very simple change that increases clarity, IMO. My preference would be to add it. But as I said, I'm not going to block on this.

vchudnov-g · 2020-10-06T21:33:21Z

src/main/java/com/google/api/codegen/GeneratorMain.java

+          .longOpt("transport")
+          .desc(
+              "List of transports to support. Valid transport names ('grpc' or 'rest') are"
+                  + " separated by '+'. Default is 'grpc'. NOTE: for now, GAPICs support only"


Please update the comment if you're not handling the list option: you're not handling the "+" separated list unless you make changes like the ones I suggested,.

vchudnov-g · 2020-10-06T21:34:08Z

src/main/java/com/google/api/codegen/gapic/GapicGeneratorApp.java

+      ToolOptions.createOption(
+          String.class,
+          "transport",
+          "List of transports to use ('rest' or 'grpc') separated by '+'. NOTE: For now"


Again, change the NOTE to be accurate: for now, we only support a single transport.

vchudnov-g · 2020-10-06T21:35:05Z

src/main/java/com/google/api/codegen/transformer/ApiCallableTransformer.java

+            populateMethodSelectors(namer, httpAttr.getPathSelectors()));
+        httpMethodView.queryParamSelectors(
+            populateMethodSelectors(namer, httpAttr.getParamSelectors()));
+        httpMethodView.bodySelectors(populateMethodSelectors(namer, httpAttr.getBodySelectors()));


vchudnov-g · 2020-10-06T21:36:42Z

src/test/java/com/google/api/codegen/gapic/ProtocGapicPluginGeneratorTest.java

@@ -53,7 +53,7 @@ public void testGenerator() {
                model.getFiles().stream().map(ProtoFile::getProto).collect(Collectors.toList()))
            // Only the file to generate a client for (don't generate dependencies)
            .addFileToGenerate("multiple_services.proto")
-            .setParameter("language=java")
+            .setParameter("language=java,transport=grpc")


vchudnov-g · 2020-10-06T21:48:46Z

src/test/java/com/google/api/codegen/testsrc/protoannotations/library.proto

@@ -86,7 +86,7 @@ service LibraryService {

  // Deletes a shelf.
  rpc DeleteShelf(DeleteShelfRequest) returns (google.protobuf.Empty) {
-    option (google.api.http) = { delete: "/v1/{name=bookShelves/*}" };
+    option (google.api.http) = { delete: "/v1/bookShelves/{name}" };


Short version: OK, but add a note.

Long version:

In our discussion today we said that the HTTP annotation is independent of resource name formatting. It just says how to substitute the fields into an HTTP pattern, not how that pattern should look like,

I think that's wrong, though: from the AIP:

URIs must use the {foo=bar/*} syntax to represent a variable that should be populated in the request proto. When extracting a resource name, the variable must include the entire resource name, not just the ID component.

That said: it's OK for the monolith to accept this input for now. I don't think microgenerators should. I would put a TODO here noting that this is non-standard, even though we won't fix it in the monolith

(And the converter emits protos in the correct format, right?)

vam-google added 7 commits August 8, 2020 16:54

feat: Implement rest-gapic prototype

7666bdf

fix: Fix DIREGAPIC path elements name format

59c36da

fix: Add transport_protocol parameter to java assembly pkg rule

45022ca

fix: Fix pathTemplate issue

7b32072

Merge remote-tracking branch 'upstream/master' into rest-gapic

e2b56c0

fix formatting

0bae1fc

Add rest transport tests

676b82c

google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Sep 10, 2020

vam-google requested review from miraleung and vchudnov-g September 10, 2020 00:05

vam-google changed the title ~~feat: REST GAPIC (REGAPIC) Support~~ feat: REST GAPIC (REGAPIC) Support for Java Sep 10, 2020

miraleung suggested changes Sep 12, 2020

View reviewed changes

vam-google added 2 commits September 11, 2020 23:47

Adjust to the corresponding gax-java PR feedback changes (toQueryPara…

63d4e38

…m -> putQueryParam, toPathParam -> putPathParam)

Address PR feedback

0f4eee3

vam-google added 2 commits September 16, 2020 23:06

increase test coverage

4395976

Merge remote-tracking branch 'upstream/master' into rest-gapic

3157ca5

vam-google requested a review from a team as a code owner September 22, 2020 21:51

vam-google requested a review from miraleung September 22, 2020 21:51

vchudnov-g suggested changes Sep 22, 2020

View reviewed changes

Address PR feedback

6045612

vam-google requested a review from vchudnov-g September 23, 2020 07:51

miraleung approved these changes Sep 25, 2020

View reviewed changes

vchudnov-g reviewed Sep 28, 2020

View reviewed changes

vam-google requested a review from vchudnov-g October 3, 2020 04:57

vchudnov-g approved these changes Oct 6, 2020

View reviewed changes

Merge remote-tracking branch 'upstream/master' into rest-gapic

9219acf

vam-google merged commit 782d11a into googleapis:master Oct 12, 2020

feat: REST GAPIC (REGAPIC) Support for Java #3275

feat: REST GAPIC (REGAPIC) Support for Java #3275

Conversation

vam-google commented Sep 10, 2020

codecov bot commented Sep 10, 2020 • edited

Codecov Report

Choose a reason for hiding this comment

vam-google Sep 16, 2020 • edited

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vam-google commented Sep 16, 2020

vchudnov-g left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vam-google Sep 23, 2020 • edited

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

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 Sep 23, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vam-google Sep 23, 2020 • edited

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

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

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 commented Sep 23, 2020

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

vchudnov-g left a 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vchudnov-g left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Sep 10, 2020 •

edited

vam-google Sep 16, 2020 •

edited

vam-google Sep 23, 2020 •

edited

vam-google Sep 23, 2020 •

edited

vam-google Sep 23, 2020 •

edited