-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Implement GenericObject - Allow GenericRecord to wrap any Java Object #10057
Conversation
pulsar-client-api/src/main/java/org/apache/pulsar/client/api/schema/GenericObject.java
Show resolved
Hide resolved
pulsar-client-api/src/main/java/org/apache/pulsar/client/api/schema/GenericRecord.java
Show resolved
Hide resolved
pulsar-client-api/src/main/java/org/apache/pulsar/client/api/schema/PrimitiveRecord.java
Outdated
Show resolved
Hide resolved
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.
@jerrypeng @lhotari I have answered to your questions and renamed PrimitiveRecord to GenericObjectWrapper.
PTAL again
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.
LGTM
pulsar-client-api/src/main/java/org/apache/pulsar/client/api/schema/GenericObjectWrapper.java
Outdated
Show resolved
Hide resolved
pulsar-client-api/src/main/java/org/apache/pulsar/client/api/schema/GenericObjectWrapper.java
Outdated
Show resolved
Hide resolved
pulsar-client-api/src/main/java/org/apache/pulsar/client/api/schema/GenericObjectWrapper.java
Outdated
Show resolved
Hide resolved
pulsar-client-api/src/main/java/org/apache/pulsar/client/api/schema/GenericObjectWrapper.java
Outdated
Show resolved
Hide resolved
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.
@sijie I have addresses your comments
PTAL again
@sijie CI passed. |
* This is an abstraction over the logical value that is store into a Message. | ||
* Pulsar decodes the payload of the Message using the Schema that is configured for the topic. | ||
*/ | ||
public interface GenericObject { |
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.
Is this interface suppose to be used as a schema type for producers or consumers, .e.g. Consumer ? If so, can you write some tests that has consumers / producers that uses this new type of Schema?
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.
Is there a reason why we need this interface as all. Shouldn't having these additional methods in GenericRecord
suffice?
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.
@jerrypeng
the idea is to let users user "GenericObject" instead of GenericRecord while dealing with "Any object", on the Consumer/Sink side, because GenericRecord for many users is related to a Struct type (like AVRO GenericRecord), and it carries "getFields()"
If so, can you write some tests that has consumers / producers that uses this new type of Schema?
I added tests about GenericObjectWrapper.
The main goal is to have Sink and having the ability to write Sinks that are not bound to a specific Schema at compile time.
…apache#10057) Contents: - introduce new high level interface GenericObject, that represents every logical value on the Pulsar topic - allow AutoConsumeSchema to deal with every Schema type - handle schema less topics with AutoConsumeSchema (return Bytes schema and byte[] payload) - rename `getNativeRecord` to `getNativeObject` How it works: - in case of non struct schema, like for primitives and for KeyValue, wrap the result into a PrimitiveRecord class, that is an implementation of GenericRecord that wraps a Object and a SchemaType, it returns an empty list of "fields" Intended audience: - allow users to implement Sink<GenericObject>, that is to write Sinks that work with every Schema type (the patch for Pulsar IO will follow up as a separate PR) ``` interface GenericObject { Object getNativeObject(); SchemaType getSchemaType(); } interface GenericRecord extends GenericObject ```
…apache#10057) Contents: - introduce new high level interface GenericObject, that represents every logical value on the Pulsar topic - allow AutoConsumeSchema to deal with every Schema type - handle schema less topics with AutoConsumeSchema (return Bytes schema and byte[] payload) - rename `getNativeRecord` to `getNativeObject` How it works: - in case of non struct schema, like for primitives and for KeyValue, wrap the result into a PrimitiveRecord class, that is an implementation of GenericRecord that wraps a Object and a SchemaType, it returns an empty list of "fields" Intended audience: - allow users to implement Sink<GenericObject>, that is to write Sinks that work with every Schema type (the patch for Pulsar IO will follow up as a separate PR) ``` interface GenericObject { Object getNativeObject(); SchemaType getSchemaType(); } interface GenericRecord extends GenericObject ```
Due to apache/pulsar#10057 We use GenericObject instead of GenericRecord to receive records.
Contents:
getNativeRecord
togetNativeObject
How it works:
Intended audience: