lhotari commented on code in PR #25179:
URL: https://github.com/apache/pulsar/pull/25179#discussion_r2719867817
##########
pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyConnection.java:
##########
@@ -436,6 +433,10 @@ private void handleBrokerConnected(DirectProxyHandler
directProxyHandler, Comman
final ByteBuf msg =
Commands.newConnected(connected.getProtocolVersion(), maxMessageSize,
connected.hasFeatureFlags() &&
connected.getFeatureFlags().isSupportsTopicWatchers());
writeAndFlush(msg);
+ // Start auth refresh task only if we are not forwarding
authorization credentials
+ if
(!service.getConfiguration().isForwardAuthorizationCredentials()) {
+ startAuthRefreshTaskIfNotStarted();
+ }
Review Comment:
It is misleading to call this "auth refresh task" in this case.
There's a clear reason why the auth refresh task has been applied only for
lookup connections since when the the connection is in
`ProxyConnectionToBroker` state, the proxy doesn't intercept the Pulsar
protocol messages.
It seems that this added logic would disconnect the connection after
`authenticationRefreshCheckSeconds`. It would be better to reflect this in the
naming of methods and not reuse the existing auth refresh logic at all.
--
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]
