+1 (binding) This proposal looks great to me. But I've got several concerns which will not affect this PIP voting.
1. You mixed compression and E2E encryption support in one proposal. I am unsure if we should split them into two parts(compression & E2E) to help make the proposal not too complex. but anyway. IMO, it's better to split the implementation PR into multiple that will help the reviewer review this PR more easily. 2. Actually, We uses a tricky way to avoid client don't compress the compressed data again(We set producer compression type to NONE. but we use the ProducerImpl to send a message entity with compressed data). IMO, it's better to introduce a public API to help client support it. but it's fine to use it to solve the web socket problem now. Best, Mattison On 22 Aug 2023 at 11:16 +0800, PengHui Li <peng...@apache.org>, wrote: > +1(binding) > > - The motivation looks good to me. The proposal will provide a real e2e > encryption solution for the WebSocket proxy > - The solution looks good to me. It will not introduce break changes and > will use public APIs as much as possible. And it will not introduce any > extra configuration. The API definition is clear and aligns with the > existing naming pattern. > - For the public API changes. We already have an encryptionKey field, but > it is key names, which not aligned with the existing definition of the > encryptionKey in the binary protocol. Instead of introducing a new one like > encryptionKeyValue, the proposal will use the existing one(encryptionKey) > and check the format on the server side. It's not so good, but better than > adding a new one to confuse users. > - The proposal quality looks good to me. It provides enough context about > what is the existing solution and what is the new solution. And provides a > comprehensive example to show what the new way looks like. > > Regards, > Penghui > > > > On Mon, Aug 21, 2023 at 5:30 PM Yubiao Feng > <yubiao.f...@streamnative.io.invalid> wrote: > > > Sorry, the PR link in the last email is ambiguous, > > https://github.com/apache/pulsar/pull/20923 is the correct one. > > > > Thanks > > Yubiao Feng > > > > On Mon, Aug 21, 2023 at 4:07 PM Yubiao Feng <yubiao.f...@streamnative.io> > > wrote: > > > > > > Hello, Guys > > > > > > > > Since there are no concerns in the discussion mail, I'd like to start > > > > voting for this PIP. > > > > > > > > The PIP link: https://github.com/apache/pulsar/pull/ > > > > <https://github.com/apache/pulsar/pull/21033>20923 > > > > <https://github.com/apache/pulsar/pull/20923> > > > > > > > > Thanks > > > > Yubiao Feng > > > > > >
