-
Notifications
You must be signed in to change notification settings - Fork 26.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Dubbo-3829] support google pb generic invocation #3975
[Dubbo-3829] support google pb generic invocation #3975
Conversation
I revisit this change today. I am hesitate to introduce protobuf dependency in dubbo-rpc-api too. Is it possible to introduce protobuf serialization impl in 'dubbo-serialization' first, then use it in GenericFilter like this: if (ProtocolUtils.isJavaGenericSerialization(generic)) {
try {
UnsafeByteArrayOutputStream os = new UnsafeByteArrayOutputStream(512);
ExtensionLoader.getExtensionLoader(Serialization.class)
.getExtension(Constants.GENERIC_SERIALIZATION_NATIVE_JAVA)
.serialize(null, os).writeObject(result.getValue()); or does 'dubbo-protostuff-serialization' meet your needs? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pls. check comments. I think we should find protobuf's serializer then use it, instead of introducing a direct dependency to protobuf.
# Conflicts: # dubbo-serialization/pom.xml
8eaf64e
to
51fe044
Compare
dubbo-common/src/main/java/org/apache/dubbo/common/Constants.java
Outdated
Show resolved
Hide resolved
dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/filter/GenericFilter.java
Show resolved
Hide resolved
dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/filter/GenericFilter.java
Outdated
Show resolved
Hide resolved
pls. also revert your change in dubbo-distrbution/pom.xml. It is unnecessary any longer. |
...b/src/main/resources/META-INF/dubbo/internal/org.apache.dubbo.common.serialize.Serialization
Outdated
Show resolved
Hide resolved
LGTM now. |
there's one CI failure: [ERROR] The project org.apache.dubbo:dubbo-serialization:2.7.2-SNAPSHOT (/home/travis/build/apache/incubator-dubbo/dubbo-serialization/pom.xml) has 1 error
[ERROR] Child module /home/travis/build/apache/incubator-dubbo/dubbo-serialization/dubbo-serialization-protobuf-json of /home/travis/build/apache/incubator-dubbo/dubbo-serialization/pom.xml does not exist
[ERROR] |
This is a quite dependent module, I think we can merge it to the main branch and keep improving it. I will give it a test once I get some time. |
What is the purpose of the change
Currently, Generic Reference cannot support Google Protobuf. With bride usage of Google PB, we are going to support it via Generic Reference. See issue #3829
Brief changelog
feedback wanted
To support Google PB Testing, GenericFilter is part of my change, also Service Metadata have to support that. If the solution is ok, i will continue on the Service Metadata part.