Cole-Greer commented on code in PR #2328:
URL: https://github.com/apache/tinkerpop/pull/2328#discussion_r1394910390
##########
gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/handler/HttpGremlinRequestEncoder.java:
##########
@@ -81,7 +90,9 @@ protected void encode(final ChannelHandlerContext
channelHandlerContext, final R
request.headers().add(HttpHeaderNames.CONTENT_TYPE,
"application/json");
request.headers().add(HttpHeaderNames.CONTENT_LENGTH,
payload.length);
request.headers().add(HttpHeaderNames.ACCEPT,
serializer.mimeTypesSupported()[0]);
-
+ if (userAgentEnabled) {
+ request.headers().add(UserAgent.USER_AGENT_HEADER_NAME,
UserAgent.USER_AGENT);
Review Comment:
`UserAgent.USER_AGENT_HEADER_NAME` (`"User-Agent"`) matches the header used
in websocket requests.
`HttpHeaderNames.USER_AGENT` (`"user-agent"`) may be preferable if we are ok
with the inconsistency.
##########
docs/src/upgrade/release-3.6.x.asciidoc:
##########
@@ -33,6 +33,9 @@ complete list of all the modifications that are part of this
release.
=== Upgrading for Providers
+The `gremlinRequestEncoder` constructor has been deprecated in favor of one
with an additional parameter `boolean userAgentEnabled`.
Review Comment:
```suggestion
The `HttpGremlinRequestEncoder` constructor has been deprecated in favor of
one with an additional parameter `boolean userAgentEnabled`.
```
##########
gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/WsAndHttpChannelizerHandler.java:
##########
@@ -62,28 +63,35 @@ public void configure(final ChannelPipeline pipeline) {
public void channelRead(final ChannelHandlerContext ctx, final Object obj)
{
final ChannelPipeline pipeline = ctx.pipeline();
if (obj instanceof HttpMessage &&
!WebSocketHandlerUtil.isWebSocket((HttpMessage)obj)) {
- // if the message is for HTTP and not websockets then this handler
injects the endpoint handler in front
- // of the HTTP Aggregator to intercept the HttpMessage. Therefore
the pipeline looks like this at start:
+ // If the message is for HTTP and not WS then this handler injects
the HTTP user-agent and endpoint handlers
+ // in front of the HTTP aggregator to intercept the HttpMessage.
+ // This replaces the WS server protocol handler so that the
pipeline initially looks like this:
//
// IdleStateHandler -> HttpResponseEncoder -> HttpRequestDecoder ->
// WsAndHttpChannelizerHandler -> HttpObjectAggregator ->
+ // WebSocketServerProtocolHandler ->
Review Comment:
Was `WebSocketServerProtocolHandler` mistakenly left out of the original
comment?
--
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]