[GitHub] [pulsar] eolivelli commented on a change in pull request #10878: Remove the unwanted dependencies in the pulsar function's instance jar

[GitBox <gi...@apache.org>]

eolivelli commented on a change in pull request #10878:
URL: https://github.com/apache/pulsar/pull/10878#discussion_r648876977



##########
File path: pulsar-functions/runtime-all/pom.xml
##########
@@ -48,7 +61,7 @@
 
     <dependency>
       <groupId>${project.groupId}</groupId>
-      <artifactId>pulsar-client-original</artifactId>

Review comment:
       I believe that is it better to have the full Pulsar Client.
   
   We should forbid the NAR file to contain Pulsar code.
   because it would introduce compatibility problems, data corruption or data loss

##########
File path: tests/docker-images/java-test-functions/src/main/java/org/apache/pulsar/tests/integration/io/TestGenericObjectSink.java
##########
@@ -56,10 +56,11 @@ public void write(Record<GenericObject> record) {
 
         if (record.getSchema().getSchemaInfo().getType() == SchemaType.KEY_VALUE) {
             // assert that we are able to access the schema (leads to ClassCastException if there is a problem)
-            KeyValueSchema kvSchema = (KeyValueSchema) record.getSchema();
-            log.info("key schema type {}", kvSchema.getKeySchema());
-            log.info("value schema type {}", kvSchema.getValueSchema());
-            log.info("key encoding {}", kvSchema.getKeyValueEncodingType());
+            // TODO need to expose KeyValueSchema was an interface in pulsar-client-api
+//            KeyValueSchema kvSchema = (KeyValueSchema) record.getSchema();

Review comment:
       This is a braking change for the new feature.
   
   Please do not change this behaviour now




-- 
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