Adding maxMessageSize config option #861
Conversation
|
@ejona86 PTAL |
|
From @carl-mastrangelo
Hmm ... I'm not sure we would ever want to support message sizes exceeding integer capacity. @louiscryan thoughts? Should we update the spec to be more clear? |
| @@ -129,6 +134,16 @@ public NettyChannelBuilder flowControlWindow(int flowControlWindow) { | |||
| return this; | |||
| } | |||
|
|
|||
| /** | |||
| * Sets the maximum message size allowed to be received on the channel. If not called, | |||
| * defaults to 100 MiB. | |||
There was a problem hiding this comment.
Could the comment reference DEFAULT_MAX_MESSAGE_SIZE instead of hard-coding 100 MiB?
There was a problem hiding this comment.
I guess not, because this is public.
There was a problem hiding this comment.
Yeah, that's why I hard-coded it :/
There was a problem hiding this comment.
This class already has several imports referencing the internal package, why not just reference it in the javadoc?
|
@nmittler LGTM The unsigned vs signed issue can be resolved later. We can change the |
|
Hmm, although I think the default 64MB limit should be enough for Android/Nano, I guess I'd better remove such limit to honor this PR? |
|
@carl-mastrangelo @ejona86 I think I've address all of the comments. PTAL. |
| @@ -99,6 +99,11 @@ | |||
| public static final String MESSAGE_ENCODING = "grpc-encoding"; | |||
|
|
|||
| /** | |||
| * The maximum uncompressed gRPC message size (in bytes). Defaults to 100 MiB. | |||
There was a problem hiding this comment.
This is only for inbound messages right?
There was a problem hiding this comment.
Cleaned up the comment.
|
@ejona86 @carl-mastrangelo PTAL. I think I've addressed everything except support for |
|
@nmittler LGTM |
|
@ejona86 do you want to take another pass or shall I cherry-pick? |
|
@nmittler LGTM |
|
Cherry-picked as 15f02ba |
Fixes #832