Cole-Greer commented on code in PR #3466:
URL: https://github.com/apache/tinkerpop/pull/3466#discussion_r3443712348


##########
CHANGELOG.asciidoc:
##########
@@ -25,6 +25,19 @@ 
image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
 [[release-4-0-0]]
 === TinkerPop 4.0.0 (Release Date: NOT OFFICIALLY RELEASED YET)
 
+* Standardized `gremlin-driver` (Java) connection options
+** Renamed the `connectionSetupTimeoutMillis` builder option to 
`connectTimeout` (default lowered from 15s to 5s) and actually wired it to 
`ChannelOption.CONNECT_TIMEOUT_MILLIS` on the connection bootstrap; it 
previously was validated but never applied. The old method is retained as a 
deprecated alias.
+** Renamed `maxConnectionPoolSize` to `maxConnections` (default 128), 
`idleConnectionTimeoutMillis` to `idleTimeout` (default 180s), and 
`resultIterationBatchSize` to `defaultBatchSize` (default 64); the old builder 
methods, accessors, and YAML keys are retained as deprecated aliases.
+** Added `traversalSource` to `RequestOptions.Builder` as the canonical name 
for `addG` (retained as a deprecated alias).
+** Added `readTimeout` (default 0/off), a per-request idle-read timeout armed 
when a request is written and reset on each inbound response chunk via a new 
`ReadTimeoutHandler` inserted after the HTTP codec, so it never fires while a 
pooled connection is idle between requests.

Review Comment:
   Overall this is a lot of changelog for work of this size, if we extend this 
across all GLVs, we could end up with 30% of the changelog for TP4 just being 
the connection options. I think it's worth trimming this as much as possible. 
The most important details users need to know is which options have been 
renamed, followed by which options have been introduced. I think most of the 
details of what each option does can be trimmed down/out.
   
   The option name should be generally self-documenting on its own, and the 
fine details should be filled in by reference documentation and javadocs 
`Cluster.Builder`, `Settings` and such.



##########
CHANGELOG.asciidoc:
##########
@@ -25,6 +25,19 @@ 
image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
 [[release-4-0-0]]
 === TinkerPop 4.0.0 (Release Date: NOT OFFICIALLY RELEASED YET)
 
+* Standardized `gremlin-driver` (Java) connection options
+** Renamed the `connectionSetupTimeoutMillis` builder option to 
`connectTimeout` (default lowered from 15s to 5s) and actually wired it to 
`ChannelOption.CONNECT_TIMEOUT_MILLIS` on the connection bootstrap; it 
previously was validated but never applied. The old method is retained as a 
deprecated alias.

Review Comment:
   We've never used sub-bullets before in the changelog to my knowledge. I 
suppose it;s not an unreasonable convention to start here.



##########
CHANGELOG.asciidoc:
##########
@@ -25,6 +25,19 @@ 
image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
 [[release-4-0-0]]
 === TinkerPop 4.0.0 (Release Date: NOT OFFICIALLY RELEASED YET)
 
+* Standardized `gremlin-driver` (Java) connection options
+** Renamed the `connectionSetupTimeoutMillis` builder option to 
`connectTimeout` (default lowered from 15s to 5s) and actually wired it to 
`ChannelOption.CONNECT_TIMEOUT_MILLIS` on the connection bootstrap; it 
previously was validated but never applied. The old method is retained as a 
deprecated alias.

Review Comment:
   We've never used sub-bullets before in the changelog to my knowledge. I 
suppose it's not an unreasonable convention to start here.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to