Re: [DISCUSS] KIP-1112: allow custom processor wrapping

2024-11-20 Thread Sophie Blee-Goldman
Thanks for the feedback all -- I have updated the KIP with everything mentioned so far, and added two static convenience methods for users to use to convert a ProcessorSupplier into a WrappedProcessorSupplier Let me know if you have any more questions or suggestions. By the way, since it was ment

Re: [DISCUSS] KIP-1112: allow custom processor wrapping

2024-11-20 Thread Sophie Blee-Goldman
Bruno -- I updated the javadocs to remove the wrapping requirement. Thank you for pointing this out Almog -- yeah that makes a lot of sense to me, especially being able to add methods for the wrapper without having to add anything to the ProcessorSupplier classes. I'm updated the KIP with this sug

Re: [DISCUSS] KIP-1112: allow custom processor wrapping

2024-11-20 Thread Almog Gavra
Hi Sophie, I was thinking it might also be good for us to return from the wrapped methods new marker interfaces like WrappedProcesserSupplier and WrappedFixedKeyProcesserSupplier. These will implement ProcesserSupplier and FixedKeyProcesserSupplier respectively with no additional methods (yet), bu

Re: [DISCUSS] KIP-1112: allow custom processor wrapping

2024-11-20 Thread Bruno Cadonna
Hi Sophie, Thanks for the details! Yeah, I think replacing the contract in the javadocs with a warning is better. Best, Bruno On 20.11.24 10:05, Sophie Blee-Goldman wrote: Have you considered to add an interface that instead of wrapping a processor adds methods for pre-processing and post-

Re: [DISCUSS] KIP-1112: allow custom processor wrapping

2024-11-20 Thread Sophie Blee-Goldman
> > Have you considered to add an interface that instead of wrapping a > processor adds methods for pre-processing and post-processing to the > processor? The problem is that we actually want to be able to access all parts of the ProcessorSupplier. This includes things like inspecting the StoreBu

Re: [DISCUSS] KIP-1112: allow custom processor wrapping

2024-11-19 Thread Bruno Cadonna
Hi Sophie, Thanks for the KIP! I am always a bit skeptical of contracts that are defined in the Java docs, but not enforced programmatically. I am talking about the contract that requires the wrapper to return the same processor supplier instance. Have you considered to add an interface that

Re: [DISCUSS] KIP-1112: allow custom processor wrapping

2024-11-19 Thread Sophie Blee-Goldman
Ah, sorry if this was unclear, to clarify I don't intend to make any changes to the TopologyConfig vs StreamsConfig stuff in this KIP. I just wanted to acknowledge that it's not ideal to have some configs in StreamsConfig which only take effect when passed into certain APIs other than the KafkaStre

Re: [DISCUSS] KIP-1112: allow custom processor wrapping

2024-11-19 Thread Lucas Brutschy
Hey, I'm a bit confused since the motivation promises a cleanup of topologyconfig/streams config, but this doesn't seem to be part of this kip. But on the whole, this change looks fine for me. Thanks for the KIP! Cheers, Lucas, Sophie Blee-Goldman schrieb am Di., 19. Nov. 2024, 00:48: > One m

Re: [DISCUSS] KIP-1112: allow custom processor wrapping

2024-11-18 Thread Sophie Blee-Goldman
One more small change -- originally I proposed to only add this config to the TopologyConfig, and not to the StreamsConfig. However while implementing a POC I noticed that TopologyConfig does not have a constructor that accepts a plain properties or config map, and only one that takes in a StreamsC

Re: [DISCUSS] KIP-1112: allow custom processor wrapping

2024-11-18 Thread Sophie Blee-Goldman
Thanks Almog! That makes sense to me, I've updated the KIP so that the ProcessorWrapper will extend Configurable but gave it a default no-op implementation so that it's optional. On Mon, Nov 18, 2024 at 1:57 PM Almog Gavra wrote: > Thanks Sophie! This KIP will certainly make it easier to impleme

Re: [DISCUSS] KIP-1112: allow custom processor wrapping

2024-11-18 Thread Almog Gavra
Thanks Sophie! This KIP will certainly make it easier to implement any kind of custom functionality across all processors in the DSL, I can imagine quite a few use cases for this. One suggestion, we should consider including a configure() method that takes in Map configs, so that it can be configu