You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/05/17 21:01:25 UTC
[GitHub] [pulsar] merlimat opened a new pull request #10616: MessageCrypto interface should not expose Netty ByteBuf class in the API
merlimat opened a new pull request #10616:
URL: https://github.com/apache/pulsar/pull/10616
### Motivation
`MessageCrypto` is included in `pulsar-client-api` and it's exposing Netty `ByteBuf` in its API.
The problem is that we are shading Netty and the classes are renamed. We must not have external libraries symbols as part of our API.
The result is that Netty symbols are exposed as `org.apache.pulsar.shade.io.netty...` and therefore are not going to be interoperable with other libraries.
Because of this, `pulsar-client-api` was also included into the shaded Jars and that brings in other issues (like missing the sources/javadocs of the PulsarClient APIs).
### Modifications
* Removed Netty dependency on `pulsar-client-api`
* Made breaking change in the MessageCrypto to use `java.nio.ByteBuffer` in API
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] jiazhai commented on pull request #10616: MessageCrypto interface should not expose Netty ByteBuf class in the API
Posted by GitBox <gi...@apache.org>.
jiazhai commented on pull request #10616:
URL: https://github.com/apache/pulsar/pull/10616#issuecomment-842896878
> Overall LGTM
>
> but the build failed
> the build failed:
>
> ```
> Error: COMPILATION ERROR :
> Error: /home/runner/work/pulsar/pulsar/pulsar-broker/src/test/java/org/apache/pulsar/client/api/SimpleProducerConsumerTest.java:[3090,42] method decrypt in interface org.apache.pulsar.client.api.MessageCrypto<MetadataT,BuilderT> cannot be applied to given types;
> required: java.util.function.Supplier,java.nio.ByteBuffer,java.nio.ByteBuffer,org.apache.pulsar.client.api.CryptoKeyReader
> found: ()->messageMetadata,io.netty.buffer.ByteBuf,org.apache.pulsar.client.api.CryptoKeyReader
> reason: actual and formal argument lists differ in length
> Error: Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:testCompile (default-testCompile) on project pulsar-broker: Compilation failure
> Error: /home/runner/work/pulsar/pulsar/pulsar-broker/src/test/java/org/apache/pulsar/client/api/SimpleProducerConsumerTest.java:[3090,42] method decrypt in interface org.apache.pulsar.client.api.MessageCrypto<MetadataT,BuilderT> cannot be applied to given types;
> Error: required: java.util.function.Supplier,java.nio.ByteBuffer,java.nio.ByteBuffer,org.apache.pulsar.client.api.CryptoKeyReader
> Error: found: ()->messageMetadata,io.netty.buffer.ByteBuf,org.apache.pulsar.client.api.CryptoKeyReader
> Error: reason: actual and formal argument lists differ in length
> Error: -> [Help 1]
> Error:
> Error: To see the full stack trace of the errors, re-run Maven with the -e switch.
> ```
looks like we need care some implementation in the tests. but this change will make the management easier. +1.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on pull request #10616: MessageCrypto interface should not expose Netty ByteBuf class in the API
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #10616:
URL: https://github.com/apache/pulsar/pull/10616#issuecomment-842776845
I have added the `release/note-required` label for this change, this means we need to highlight in the release note since this introduced a break change.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli merged pull request #10616: MessageCrypto interface should not expose Netty ByteBuf class in the API
Posted by GitBox <gi...@apache.org>.
eolivelli merged pull request #10616:
URL: https://github.com/apache/pulsar/pull/10616
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org