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

Bump gRPC version. #56

Merged
merged 2 commits into from
Aug 14, 2015
Merged

Bump gRPC version. #56

merged 2 commits into from
Aug 14, 2015

Conversation

deflaux
Copy link
Contributor

@deflaux deflaux commented Aug 14, 2015

No description provided.

@@ -98,7 +98,6 @@ private static Channel getGenomicsChannel() throws SSLException {

Channel channel = NettyChannelBuilder.forAddress("genomics.googleapis.com", 443)
.negotiationType(NegotiationType.TLS)
.streamWindowSize(1000000)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have verified this is no longer necessary for performance, I take it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ssgross There is a different method to set that value now. See https://github.com/grpc/grpc-java/blob/master/netty/src/main/java/io/grpc/netty/NettyChannelBuilder.java#L126

But since the default is 1048576, which is roughly equivalent to what was in the file, I took it out. See https://github.com/grpc/grpc-java/blob/master/netty/src/main/java/io/grpc/netty/NettyChannelBuilder.java#L54

Let me know if you think we should continue to set it explicitly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, that's fine, the default used to be much lower so I guess they raised it.

@ssgross
Copy link
Contributor

ssgross commented Aug 14, 2015

LGTM modulo streamWindowSize comment.

These come in through the grpc-java dependency.
@deflaux
Copy link
Contributor Author

deflaux commented Aug 14, 2015

@ssgross I added one more commit that removes some redundant code per @elmer-garduno. code-gen, unit tests, and integration tests all pass. PTAL.

@ssgross
Copy link
Contributor

ssgross commented Aug 14, 2015

LGTM

deflaux added a commit that referenced this pull request Aug 14, 2015
@deflaux deflaux merged commit b0bf45a into googlegenomics:master Aug 14, 2015
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.

None yet

2 participants