RobertIndie commented on PR #19540: URL: https://github.com/apache/pulsar/pull/19540#issuecomment-1437826208
Hi, @michaeljmarshall . Thanks for your comment. > The client version is still filtered out for the Java client for the same reason that the Pulsar proxy client version is filtered out. Do you have reproducible steps? It seems to work fine for me. The java client should always set the client_version here: https://github.com/apache/pulsar/blob/954f406fab03ce8858883335ce6d5a676bb45c92/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ClientCnx.java#L295-L297 It will not use `Pulsar Client` as the value here: https://github.com/apache/pulsar/blob/954f406fab03ce8858883335ce6d5a676bb45c92/pulsar-common/src/main/java/org/apache/pulsar/common/protocol/Commands.java#L198 > In my opinion, the right solution is to consider removing the logic that filters out versions with the " " and then to make sure that all official clients have a meaningful version string. Agree. I thought that it was used for testing. But it doesn't seem to be useful now. I think we can remove this logic if there is no other objections. > Further, it would be helpful to know when a connection is being proxied, which brings into scope this PR. Instead of using just one clientVersion, I think we should add a new protobuf field specifically for the proxyVersion. I would also make the restriction that a client cannot set this field unless it authenticates as a proxy (assuming authentication/authorization is enabled). I'm +1 for this. But from my understanding, this is out of the scope of this PR discussion. This is adding a new feature to show the version of the proxy that the client connects. > Further, I think it is a stretch to call this a bug. I view it as just a missing feature. We could only get the client version from the broker if the client connects to the broker directly instead of through the proxy. It results in different behavior when using the proxy. I think it's a bug because the proxy should be transparent to users for the `client_version`. -- 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]
