merlimat commented on a change in pull request #10878:
URL: https://github.com/apache/pulsar/pull/10878#discussion_r650189859
##########
File path:
pulsar-io/kafka/src/main/java/org/apache/pulsar/io/kafka/AvroSchemaCache.java
##########
@@ -65,7 +66,7 @@ public AvroSchemaCache(SchemaRegistryClient
schemaRegistryClient) {
org.apache.avro.Schema schema =
schemaRegistryClient.getById(schemaId);
String definition = schema.toString(false);
log.info("Schema {} definition {}", schemaId, definition);
- SchemaInfo schemaInfo = SchemaInfo.builder()
+ SchemaInfo schemaInfo = SchemaInfoImpl.builder()
Review comment:
I think this was already pulling in pulsar-common
##########
File path:
pulsar-io/kafka/src/main/java/org/apache/pulsar/io/kafka/AvroSchemaCache.java
##########
@@ -65,7 +66,7 @@ public AvroSchemaCache(SchemaRegistryClient
schemaRegistryClient) {
org.apache.avro.Schema schema =
schemaRegistryClient.getById(schemaId);
String definition = schema.toString(false);
log.info("Schema {} definition {}", schemaId, definition);
- SchemaInfo schemaInfo = SchemaInfo.builder()
+ SchemaInfo schemaInfo = SchemaInfoImpl.builder()
Review comment:
> is this Source adding pulsar-common in the dependencies ?
Yes, that is the case
##########
File path:
pulsar-io/kafka/src/main/java/org/apache/pulsar/io/kafka/AvroSchemaCache.java
##########
@@ -65,7 +66,7 @@ public AvroSchemaCache(SchemaRegistryClient
schemaRegistryClient) {
org.apache.avro.Schema schema =
schemaRegistryClient.getById(schemaId);
String definition = schema.toString(false);
log.info("Schema {} definition {}", schemaId, definition);
- SchemaInfo schemaInfo = SchemaInfo.builder()
+ SchemaInfo schemaInfo = SchemaInfoImpl.builder()
Review comment:
I think the current state is OK for now. All functionality is working
with current approach.
For a more general approach I think we should add a mechanism to detect the
classloader information and identify we are in "pulsar function mode".
Therefore, in `DefaultImplementation` class, we should be able to directly use
the appropriate classloader, irrespective of where the access is coming from.
I believe this is something we should be targeting for 2.9.
--
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:
[email protected]
