acrites commented on code in PR #37631:
URL: https://github.com/apache/beam/pull/37631#discussion_r2933123396


##########
sdks/java/core/src/main/java/org/apache/beam/sdk/util/construction/ModelCoderRegistrar.java:
##########


Review Comment:
   I'll see if I can make something like that work. Here's the current 
implementation though, which might make that tricky. In particular, there are 
potentially many different CoderRegistrar's:
   
   * When trying to translate a coder into a proto, we ask all of the 
registered CoderRegistrars to give us their maps of known coders and combine 
them into one big map. This map is currently cached so we only have to do this 
once.
   * We then use this map to look up translators for coders as needed. If the 
coder can't be found in the map, we use the Java custom coder wrapper.
   
   I think I've already plumbed a PipelineOptions object (via the 
TranslationContexts and/or SdkComponents objects) and I think I still need to 
do all that plumbing with either solution.
   
   If, in the new solution I want to only ask for a single CoderTranslator at a 
time with a `getCoderTranslator(Class)` method, I'd have to loop over all 
registered CoderRegistrar's each time until I find one that knows about the 
coder type I'm asking for. I'd need to make the PipelineOptions part of the 
cache results if I want to still be able to cache them since, in theory we 
could be asking for a translator for the same coder, but with different 
options. But by doing this, we could pin the CoderTranslation's behavior in its 
construction rather than when translating.
   
   Did I understand your idea correctly?



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