Great discussion. Derivative clients are an important consideration for our discussion around capturing the version information.
Is there any way we can avoid giving regular users easy access to this field via the ClientBuilder while still letting libraries add their own suffix? We cannot prevent a custom library from serializing whatever they would like in the field, but making this field easily available to application code could be confusing and might decrease the value of the version information. In my mind, the goal of the version string is to give a weak signal that helps operators debug client related issues. If we do leave this field exposed to the application code, I think we should consider putting a character limit on the field to prevent descriptions that are too long. Thanks, Michael On Thu, Mar 9, 2023 at 8:28 PM Yunze Xu <y...@streamnative.io.invalid> wrote: > > 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! > > > > > > > > > > > > > > >
