Integrating AWS CRT 0.5.1 into aws-crt-dev-preview #1769
Conversation
Factoring out EventLoopGroup/HostResolver from client constructor, fixing bug reported by Sonar
| @@ -43,9 +47,13 @@ public void setup() { | |||
| continue; | |||
| } | |||
|
|
|||
| int numThreads = 1; | |||
| eventLoopGroup = new EventLoopGroup(numThreads); | |||
There was a problem hiding this comment.
I"m not sure what the purposes of these changes is. Care to enlighten me?
There was a problem hiding this comment.
The client doesn't create its own event loop group anymore, as people felt it was a bit awkward for them to create such a heavy object. Because of that, we don't have the .eventLoopSize(1) builder change that we used to have on the client, and this is meant to be the replacement of that. (The numThreads variable is just meant to be more explicit about what's being passed in, and could arguably be taken out or made final.)
| .withMaxConnections(maxConnectionsPerEndpoint) | ||
| .withManualWindowManagement(manualWindowManagement); | ||
|
|
||
| return HttpClientConnectionManager.create(options); |
There was a problem hiding this comment.
create is a wierd verb for a builder pattern
There was a problem hiding this comment.
Wouldn't that be a change we would have to make on the CRT side?
| */ | ||
| Builder verifyPeer(boolean verifyPeer); | ||
|
|
||
| /** | ||
| * If set to true, then the TCP read back pressure mechanism will be enabled. |
There was a problem hiding this comment.
make sure to call out if this is ever set to true the user is responsible for calling updateWindow on the stream object.
There was a problem hiding this comment.
sorry, incrementWindow()
There was a problem hiding this comment.
I'll update the comment!
|
SonarCloud Quality Gate failed.
|
Description
Testing
mvn clean install
Types of changes
Checklist
mvn installsucceedsLicense