I've updated this proposal to retain the original client version string. I'd rather use the "description" term, which indicates the client version has extra description in addition to the client version string.
Thanks, Yunze On Fri, Mar 10, 2023 at 10:11 AM Yunze Xu <y...@streamnative.io> wrote: > > Hi Zike, > > Good suggestion. I agree with this approach. Maybe we can name it as > `subVersionString` to indicate that? > > Thanks, > Yunze > > On Thu, Mar 9, 2023 at 3:14 PM Zike Yang <z...@apache.org> wrote: > > > > Hi Yunze, > > > > > I have changed this proposal to just add a config to `ClientBuilder`. > > > > I propose to add a field named `clientVersionSuffix` rather than the > > `clientVersion`. As I said before: > > https://lists.apache.org/thread/g0128l85fkcmw4821188mjjznxbo4lhd > > > > This is helpful for debugging. Especially for the case of the Nodejs > > client in which users can compile the C++ client on their own. This > > way, we can know exactly which underlying C++ client version the user > > uses. > > > > Thanks, > > Zike Yang > > > > On Wed, Mar 8, 2023 at 5:17 PM Yunze Xu <y...@streamnative.io.invalid> > > wrote: > > > > > > Hi, > > > > > > I have changed this proposal to just add a config to `ClientBuilder`. > > > And here is the demo implementation: > > > https://github.com/BewareMyPower/pulsar/pull/21/files > > > > > > PTAL again. > > > > > > Thanks, > > > Yunze > > > > > > On Sat, Mar 4, 2023 at 10:39 PM Yunze Xu <y...@streamnative.io> wrote: > > > > > > > > Hi Enrico, > > > > > > > > Thanks for your suggestion. It makes sense to me. I will think again > > > > and modify this proposal. > > > > > > > > Hi Tison, > > > > > > > > I mentioned the C++ client because the initial motivation is to solve > > > > the issue for the Python client and Node.js client. But after thinking > > > > for a while, I believe it's more general for clients of other > > > > languages, including Java. And this proposal is only for the Java > > > > client. > > > > > > > > Thanks, > > > > Yunze > > > > > > > > On Sat, Mar 4, 2023 at 1:42 PM tison <wander4...@gmail.com> wrote: > > > > > > > > > > I agree with Enrico that it's better to have a config option. > > > > > > > > > > Also, we cannot simply replace the PulsarVersion call with the > > > > > DynamicPulsarVersion call because the client version string is now > > > > > constructed as: > > > > > > > > > > String.format("Pulsar-Java-v%s", PulsarVersion.getVersion()) > > > > > > > > > > It's a config of client version string, not pulsar version. > > > > > > > > > > Moreover, in your proposal, you mention the case of client c++ at > > > > > first, > > > > > but don't talk about it later. Is the scope of this proposal in the > > > > > Java > > > > > client only? > > > > > > > > > > Best, > > > > > tison. > > > > > > > > > > > > > > > Enrico Olivelli <eolive...@gmail.com> 于2023年3月4日周六 06:38写道： > > > > > > > > > > > Yunze, > > > > > > > > > > > > Il Ven 3 Mar 2023, 12:31 Yunze Xu <y...@streamnative.io.invalid> ha > > > > > > scritto: > > > > > > > > > > > > > Hi all, > > > > > > > > > > > > > > Based on the previous discussion [1], I created a proposal to > > > > > > > support > > > > > > > configuring client version at SDK level: > > > > > > > https://github.com/apache/pulsar/issues/19705 > > > > > > > > > > > > > > I've added more explanations in the motivation part, let's use > > > > > > > this > > > > > > > PIP as a subsequent discussion of [1]. > > > > > > > > > > > > > > BTW, there is a PR [2] in the pulsar-client-cpp repo because the > > > > > > > motivation is more meaningful for the C++ client. > > > > > > > > > > > > > > > > > > > I understand well this problem, we have it for the cited clients > > > > > > but I also > > > > > > see the same issue for other libraries based on the Java client, > > > > > > like the > > > > > > official Apache Pulsar Reactive client. > > > > > > > > > > > > I also see this problem in Startlight for JMS that is a JMS client > > > > > > for > > > > > > Pulsar that is based on the Java client. > > > > > > > > > > > > While I agree on the problem and on the solution I think that a > > > > > > static > > > > > > field is not enough, we have some problems: > > > > > > > > > > > > 1) there may be multiple usages of the Java client in the same JVM, > > > > > > and you > > > > > > want each client to report correctly its version > > > > > > > > > > > > 2) we would need to use the Java security Manager in order to > > > > > > prevent > > > > > > malicious code to modify the version or some other mechanism to > > > > > > prevent > > > > > > overriding the version. > > > > > > > > > > > > I believe that in the case of the Java client is is easier to add a > > > > > > configuration entry to the Pulsar Client Configuration. That would > > > > > > become a > > > > > > field in the JavaClient. So each instance can declare its version > > > > > > and also > > > > > > malicious code won't be able ti easily tweak the version (because > > > > > > it won't > > > > > > be a simple static method call) > > > > > > > > > > > > Enrico > > > > > > > > > > > > > > > > > > > > > > > > > [1] > > > > > > > https://lists.apache.org/thread/n59k537fhthjnzkfxtc2p4zk4l0cv3mp > > > > > > > [2] https://github.com/apache/pulsar-client-cpp/pull/208 > > > > > > > > > > > > > > Thanks! > > > > > > > > > > > > >
