Skip to content
This repository has been archived by the owner on Sep 26, 2023. It is now read-only.

switch from NettyChannelBuilder to ManagedChannelBuilder (#208) #215

Merged
merged 1 commit into from
Mar 8, 2017

Conversation

neozwu
Copy link
Contributor

@neozwu neozwu commented Mar 1, 2017

Use ManagedChannelBuilder to replace NettyChannelBuilder, since the later is experimental. The ManagedChannelBuilder at runtime will load NettyChannelBuilder on all platforms except for Android, in which case OkHttpChannelBuilder will be used.

@codecov-io
Copy link

codecov-io commented Mar 1, 2017

Codecov Report

Merging #215 into master will increase coverage by 0.02%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##             master     #215      +/-   ##
============================================
+ Coverage     70.52%   70.54%   +0.02%     
  Complexity      492      492              
============================================
  Files            69       69              
  Lines          2551     2550       -1     
  Branches        269      269              
============================================
  Hits           1799     1799              
+ Misses          653      652       -1     
  Partials         99       99
Impacted Files Coverage Δ Complexity Δ
...gle/api/gax/grpc/InstantiatingChannelProvider.java 55.43% <0%> (+0.59%) 11 <0> (ø)

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 38c0546...67efe6d. Read the comment docs.

@@ -116,8 +115,8 @@ private ManagedChannel createChannel(Executor executor) throws IOException {
interceptors.add(new ClientAuthInterceptor(credentialsProvider.getCredentials(), executor));
interceptors.add(new HeaderInterceptor(serviceHeader()));

return NettyChannelBuilder.forAddress(serviceAddress, port)
.negotiationType(NegotiationType.TLS)
//By default ManagedChannel uses TLS negotiationType

This comment was marked as spam.

@garrettjonesgoogle
Copy link
Member

Have you run the tests in api-client-staging with this change?

@shinfan
Copy link
Contributor

shinfan commented Mar 1, 2017

@neozwu To run the staging tests with this PR:

  1. Locally build and install gax-java: ./gradlew clean install
  2. Go to api-client-staging and switch the gax dependency to the local SNAPSHOT version:
    https://github.com/googleapis/api-client-staging/blob/master/generated/java/build.gradle#L34
  3. Run tests: ./gradlew clean test

Hope this helps! Please let me know if anything does not work for you.

@neozwu neozwu force-pushed the switch-to-managedchannelbuilder branch from be34ded to 67efe6d Compare March 8, 2017 01:10
@neozwu
Copy link
Contributor Author

neozwu commented Mar 8, 2017

@garrettjonesgoogle Testing against api-client-staging all passed. PTAL

@garrettjonesgoogle
Copy link
Member

LGTM

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants