robertwb commented on a change in pull request #17101:
URL: https://github.com/apache/beam/pull/17101#discussion_r832609421



##########
File path: 
sdks/java/extensions/python/src/main/java/org/apache/beam/sdk/extensions/python/ExternalPythonTransform.java
##########
@@ -33,25 +40,169 @@
 import org.apache.beam.sdk.values.POutput;
 import org.apache.beam.sdk.values.Row;
 import org.apache.beam.sdk.values.TupleTag;
+import org.apache.beam.vendor.bytebuddy.v1_11_0.net.bytebuddy.ByteBuddy;

Review comment:
       The downside is that it's way more complicated than need be, and likely 
more brittle too (using bytebuddy, invoking class loaders, etc.) It also 
accumulates garbage in global state (e.g. the classloader, schema registry). 
The POJO -> schema conversions are complicated because they handle all kinds of 
variants of how fields are named, annotations, private members vs autovalues vs 
getter/setters, etc. which are not relevant here. 
   
   What about
   
   ```
     Schema schemaFromMap(Map<String, Object> fields) {
       Schema.Builder builder = Schema.builder();
       for (Map.Entry<String, Object> entry : fields.entrySet()) {
         Schema.builder().addField(
             entry.getKey(),
             StaticSchemaInference.fieldFromType(
                 TypeDescriptor.of(entry.getValue().getClass()),
                 JavaFieldSchema.JavaFieldTypeSupplier.INSTANCE));
       }
       return builder.build();
     }
   ```




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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to