You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pulsar.apache.org by Yunze Xu <yz...@streamnative.io.INVALID> on 2023/03/03 11:29:53 UTC
[DISCUSS] PIP-254: Support configuring client version at SDK level
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.
[1] https://lists.apache.org/thread/n59k537fhthjnzkfxtc2p4zk4l0cv3mp
[2] https://github.com/apache/pulsar-client-cpp/pull/208
Thanks!
Re: [DISCUSS] PIP-254: Support configuring client version at SDK level
Posted by Yunze Xu <yz...@streamnative.io.INVALID>.
Hi Zike and Michael,
I agree that we can move this method to `ClientBuilderImpl`. Generally
users should not set this description, the client version string is
used to identify a specific implementation of the Pulsar API. If there
is a strong demand to configure the description for the API layer
(`PulsarClient`), we can open a new PIP later.
So I changed the PIP to add this method for `ClientBuilderImpl`. And
still, there is a length limit for it.
Thanks,
Yunze
On Mon, Mar 13, 2023 at 3:47 PM Zike Yang <zi...@apache.org> wrote:
>
> > I'd rather use the "description" term, which indicates the
> client version has extra description in addition to the client version
> string.
>
> The `description` term and 64 length limit all make sense to me.
>
> > 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?
>
> I think we need to consider this problem. Is it possible to expose
> this field to the `ClientBuilderImpl` instead of the `ClientBuilder`?
> This will make users not easy to access this field. The `description`
> still seems confusing for the user. Are there any strong cases where
> the field `description` makes sense to the general user and not just
> the library developer?
>
> Thanks,
> Zike Yang
>
> On Mon, Mar 13, 2023 at 9:31 AM Yunze Xu <yz...@streamnative.io.invalid> wrote:
> >
> > > I think we
> > > should consider putting a character limit on the field to prevent
> > > descriptions that are too long.
> >
> > Good suggestion. A long description string is unnecessary and could be
> > used as malicious code. What do you think of limiting the length to
> > 64? I think it's long enough.
> >
> > Thanks,
> > Yunze
> >
> > On Fri, Mar 10, 2023 at 12:52 PM Michael Marshall <mm...@apache.org> wrote:
> > >
> > > 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 <yz...@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 <yz...@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 <zi...@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 <yz...@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 <yz...@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 <wa...@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 <eo...@gmail.com> 于2023年3月4日周六 06:38写道：
> > > > > > > > >
> > > > > > > > > > Yunze,
> > > > > > > > > >
> > > > > > > > > > Il Ven 3 Mar 2023, 12:31 Yunze Xu <yz...@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!
> > > > > > > > > > >
> > > > > > > > > >
Re: [DISCUSS] PIP-254: Support configuring client version at SDK level
Posted by Zike Yang <zi...@apache.org>.
> I'd rather use the "description" term, which indicates the
client version has extra description in addition to the client version
string.
The `description` term and 64 length limit all make sense to me.
> 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?
I think we need to consider this problem. Is it possible to expose
this field to the `ClientBuilderImpl` instead of the `ClientBuilder`?
This will make users not easy to access this field. The `description`
still seems confusing for the user. Are there any strong cases where
the field `description` makes sense to the general user and not just
the library developer?
Thanks,
Zike Yang
On Mon, Mar 13, 2023 at 9:31 AM Yunze Xu <yz...@streamnative.io.invalid> wrote:
>
> > I think we
> > should consider putting a character limit on the field to prevent
> > descriptions that are too long.
>
> Good suggestion. A long description string is unnecessary and could be
> used as malicious code. What do you think of limiting the length to
> 64? I think it's long enough.
>
> Thanks,
> Yunze
>
> On Fri, Mar 10, 2023 at 12:52 PM Michael Marshall <mm...@apache.org> wrote:
> >
> > 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 <yz...@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 <yz...@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 <zi...@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 <yz...@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 <yz...@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 <wa...@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 <eo...@gmail.com> 于2023年3月4日周六 06:38写道：
> > > > > > > >
> > > > > > > > > Yunze,
> > > > > > > > >
> > > > > > > > > Il Ven 3 Mar 2023, 12:31 Yunze Xu <yz...@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!
> > > > > > > > > >
> > > > > > > > >
Re: [DISCUSS] PIP-254: Support configuring client version at SDK level
Posted by Yunze Xu <yz...@streamnative.io.INVALID>.
> I think we
> should consider putting a character limit on the field to prevent
> descriptions that are too long.
Good suggestion. A long description string is unnecessary and could be
used as malicious code. What do you think of limiting the length to
64? I think it's long enough.
Thanks,
Yunze
On Fri, Mar 10, 2023 at 12:52 PM Michael Marshall <mm...@apache.org> wrote:
>
> 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 <yz...@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 <yz...@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 <zi...@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 <yz...@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 <yz...@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 <wa...@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 <eo...@gmail.com> 于2023年3月4日周六 06:38写道：
> > > > > > >
> > > > > > > > Yunze,
> > > > > > > >
> > > > > > > > Il Ven 3 Mar 2023, 12:31 Yunze Xu <yz...@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!
> > > > > > > > >
> > > > > > > >
Re: [DISCUSS] PIP-254: Support configuring client version at SDK level
Posted by Michael Marshall <mm...@apache.org>.
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 <yz...@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 <yz...@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 <zi...@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 <yz...@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 <yz...@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 <wa...@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 <eo...@gmail.com> 于2023年3月4日周六 06:38写道：
> > > > > >
> > > > > > > Yunze,
> > > > > > >
> > > > > > > Il Ven 3 Mar 2023, 12:31 Yunze Xu <yz...@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!
> > > > > > > >
> > > > > > >
Re: [DISCUSS] PIP-254: Support configuring client version at SDK level
Posted by Yunze Xu <yz...@streamnative.io.INVALID>.
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 <yz...@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 <zi...@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 <yz...@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 <yz...@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 <wa...@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 <eo...@gmail.com> 于2023年3月4日周六 06:38写道：
> > > > >
> > > > > > Yunze,
> > > > > >
> > > > > > Il Ven 3 Mar 2023, 12:31 Yunze Xu <yz...@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!
> > > > > > >
> > > > > >
Re: [DISCUSS] PIP-254: Support configuring client version at SDK level
Posted by Yunze Xu <yz...@streamnative.io.INVALID>.
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 <zi...@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 <yz...@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 <yz...@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 <wa...@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 <eo...@gmail.com> 于2023年3月4日周六 06:38写道：
> > > >
> > > > > Yunze,
> > > > >
> > > > > Il Ven 3 Mar 2023, 12:31 Yunze Xu <yz...@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!
> > > > > >
> > > > >
Re: [DISCUSS] PIP-254: Support configuring client version at SDK level
Posted by Zike Yang <zi...@apache.org>.
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 <yz...@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 <yz...@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 <wa...@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 <eo...@gmail.com> 于2023年3月4日周六 06:38写道：
> > >
> > > > Yunze,
> > > >
> > > > Il Ven 3 Mar 2023, 12:31 Yunze Xu <yz...@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!
> > > > >
> > > >
Re: [DISCUSS] PIP-254: Support configuring client version at SDK level
Posted by Yunze Xu <yz...@streamnative.io.INVALID>.
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 <yz...@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 <wa...@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 <eo...@gmail.com> 于2023年3月4日周六 06:38写道：
> >
> > > Yunze,
> > >
> > > Il Ven 3 Mar 2023, 12:31 Yunze Xu <yz...@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!
> > > >
> > >
Re: [DISCUSS] PIP-254: Support configuring client version at SDK level
Posted by Yunze Xu <yz...@streamnative.io.INVALID>.
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 <wa...@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 <eo...@gmail.com> 于2023年3月4日周六 06:38写道：
>
> > Yunze,
> >
> > Il Ven 3 Mar 2023, 12:31 Yunze Xu <yz...@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!
> > >
> >
Re: [DISCUSS] PIP-254: Support configuring client version at SDK level
Posted by tison <wa...@gmail.com>.
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 <eo...@gmail.com> 于2023年3月4日周六 06:38写道：
> Yunze,
>
> Il Ven 3 Mar 2023, 12:31 Yunze Xu <yz...@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!
> >
>
Re: [DISCUSS] PIP-254: Support configuring client version at SDK level
Posted by Enrico Olivelli <eo...@gmail.com>.
Yunze,
Il Ven 3 Mar 2023, 12:31 Yunze Xu <yz...@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!
>