flowchartsman commented on PR #20587:
URL: https://github.com/apache/pulsar/pull/20587#issuecomment-1602006583

   Unfortunately, this doesn't do what you want. 
   
   In essence, this monkey-patches the (already not great) `NewOutputMessage` 
functionality, which is a 
[closure](https://github.com/apache/pulsar/blob/master/pulsar-function-go/pf/instance.go#L72-L78)
 that creates a _new_producer.
   
   `NewOutputMessage` was added in an [attempt to replicate the multi-topic 
dispatch](https://github.com/apache/pulsar/pull/8327) provided by the context 
object in the other two language frameworks. In addition to creating a _new_ 
producer for every call (making it inefficient and relatively wasteful), 
`NewOutputMessage` actually has nothing to do with the primary producer the SDK 
creates to process messages output by your closure.
   
   The primary SDK producer is stored in an [unexported 
field](https://github.com/apache/pulsar/blob/master/pulsar-function-go/pf/instance.go#L42)
 of the (also unexported) `pf.goInstance` type.  
   
   In order to do this as an importer, you'd have to expose `pf.goInstance` to 
`FunctionContext`, and add a method that swapped out the producer, but I feel 
like that's asking for trouble, since it doesn't have any use beyond mocking 
the producer, which itself is probably overkill, since you can just unit test 
the function you're passing to `pf.Start`. If you are ourputting messages, this 
function should already be returning an output you can test along with 
(ideally) an error.  All you really have to do is test this function.


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