Refactoring to allow multiple transports in GAPIC clients by garrettjonesgoogle · Pull Request #2156 · googleapis/google-cloud-java

garrettjonesgoogle · 2017-06-16T04:49:24Z

No description provided.

garrettjonesgoogle · 2017-07-13T19:18:49Z

This is ready to review; it is only WIP because gax-java won't be released until this PR is approved.

garrettjonesgoogle · 2017-07-17T16:06:11Z

gax-java and toolkit are approved, PTAL here

michaelbausor · 2017-07-17T16:49:13Z

LGTM, others should approve also

shinfan

Overall looks good to me. Just a few comments & questions

Sign in to view

 }

+ /**
+ * Returns a builder for this class with recommened defaults for API methods, and the given


Sign in to view

-settingsBuilder =
+ ImmutableMap.Builder<String, RetrySettings> definitions = ImmutableMap.builder();
+ RetrySettings settings = null;
+settings =


Sign in to view

+// AUTO-GENERATED DOCUMENTATION AND CLASS
+@Generated("by GAPIC v0.0.5")
+@BetaApi
+public abstract class DlpServiceStub implements BackgroundResource {


Sign in to view

+
+ /**
+ * Initiates an orderly shutdown in which preexisting calls continue but new calls are immediately
+ * cancelled.


Sign in to view

+
+ @Override
+ public void shutdown() {
+ backgroundResources.shutdown();


Sign in to view

+// AUTO-GENERATED DOCUMENTATION AND CLASS
+@Generated("by GAPIC v0.0.5")
+@BetaApi
+public class GrpcDlpServiceStub extends DlpServiceStub {


Sign in to view

+ ListRootCategoriesRequest.getDefaultInstance()),
+ io.grpc.protobuf.ProtoUtils.marshaller(
+ ListRootCategoriesResponse.getDefaultInstance())));
+ private final BackgroundResource backgroundResources;


Sign in to view

+
+ /**
+ * Constructs an instance of GrpcDlpServiceStub, using the given settings. This is protected so
+ * that it easy to make a subclass, but otherwise, the static factory methods should be preferred.


pongad · 2017-07-19T06:10:36Z

The code LGTM, assuming @shinfan 's concerns are addressed.

I'm not sure if this could cause a performance hit because we're wrapping more stuff. Though I'm not sure if we should block on this.

garrettjonesgoogle · 2017-07-19T16:25:50Z

Addressed Shin's comments, PTAL @shinfan

garrettjonesgoogle · 2017-07-19T16:54:13Z

@pongad I don't think the extra stack frame matters. I think what does matter is that gax was refactored a bit to make this happen, so allocation characteristics may have changed at that level. Like you say, I don't think possible performance changes should impact this PR; however, I think it might be worth doing a perf test before we do a release of google-cloud-java.

shinfan

LGTM

pongad · 2017-07-19T23:53:42Z

@garrettjonesgoogle I agree. That's what I meant; I just didn't think of this when looking at the gax PR. I'll work on measurement when I get cycles.

…d-java

coveralls · 2017-07-20T19:04:51Z

Changes Unknown when pulling d5575d8 on garrettjonesgoogle:master into ** on GoogleCloudPlatform:master**.

…v1.43.3 (#2156) * deps: update dependency com.google.http-client:google-http-client to v1.43.3 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

#2156) * chore: Update generation configuration at Thu Aug 29 02:21:05 UTC 2024 * chore: Update generation configuration at Fri Aug 30 02:21:44 UTC 2024 * chore: Update generation configuration at Sat Aug 31 02:20:41 UTC 2024 * chore: Update generation configuration at Wed Sep 4 02:21:45 UTC 2024 * chore: Update generation configuration at Thu Sep 5 02:22:12 UTC 2024 * chore: Update generation configuration at Fri Sep 6 02:22:02 UTC 2024 * chore: Update generation configuration at Sat Sep 7 02:20:17 UTC 2024 * chore: Update generation configuration at Sun Sep 8 02:25:47 UTC 2024 * chore: Update generation configuration at Tue Sep 10 02:23:16 UTC 2024

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jun 16, 2017

garrettjonesgoogle force-pushed the master branch 2 times, most recently from 14d346c to 8b036cbCompare July 6, 2017 19:49

[WIP] Refactoring to allow multiple transports
3c848bf

garrettjonesgoogle force-pushed the master branch from 8b036cb to 3c848bfCompare July 13, 2017 19:17

garrettjonesgoogle assigned shinfan, pongad and michaelbausor Jul 13, 2017

garrettjonesgoogle requested review from michaelbausor, pongad and shinfan July 17, 2017 16:06

shinfan reviewed Jul 18, 2017
View reviewed changes

Addressing PR feedback
a32333a

shinfan approved these changes Jul 19, 2017
View reviewed changes

garrettjonesgoogle added 2 commits July 20, 2017 11:27

Merge branch 'master' ofhttps://github.com/GoogleCloudPlatform/gclou…
b9134dd
…d-java

Using released gax versions
d5575d8

garrettjonesgoogle changed the title ~~[WIP] Refactoring to allow multiple transports~~Refactoring to allow multiple transports in GAPIC clientsJul 20, 2017

garrettjonesgoogle merged commit 0e4ed46 into googleapis:masterJul 20, 2017

garrettjonesgoogle mentioned this pull request Jul 27, 2017
Adding @BetaApi to particular exception constructors #2286
Merged

theacodes unassigned shinfan, pongad and michaelbausor Sep 28, 2018

meltsufin pushed a commit that referenced this pull request May 2, 2026
deps: update googleapis/sdk-platform-java action to v2.60.0 (#2156)
fda3068

benjaminp pushed a commit to benjaminp/google-cloud-java that referenced this pull request Jul 13, 2026
chore(main): release 2.35.2-SNAPSHOT (googleapis#2156)
0b03bac
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>

Uh oh!

Conversation

garrettjonesgoogle commented Jun 16, 2017

Uh oh!

garrettjonesgoogle commented Jul 13, 2017

Uh oh!

garrettjonesgoogle commented Jul 17, 2017

Uh oh!

michaelbausor commented Jul 17, 2017

Uh oh!

shinfan left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

pongad commented Jul 19, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

garrettjonesgoogle commented Jul 19, 2017

Uh oh!

garrettjonesgoogle commented Jul 19, 2017

Uh oh!

shinfan left a comment

Choose a reason for hiding this comment

Uh oh!

pongad commented Jul 19, 2017

Uh oh!

coveralls commented Jul 20, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

pongad commented Jul 19, 2017•
edited
Loading