Hi Tom, thanks for your detailed explanation! I may have mis-phrased my mail a little bit regarding the definition of conditions and caused you extra effort with that, sorry! I understood the not condition and its relationship to the has-header condition, I was just wondering whether the "<conditionName>" in "transforms.conditionalExtract.condition.<conditionName>" is really optional or if you just forgot to add it to the other example.
Regarding the topic of more complex transformations I am torn to be honest. When I am with customers I usually tell them about Connect being a fairly simple tool to move data from/to Kafka. In the next sentence I'll usually tell them that they'll probably reach its limits very quickly if they start using it for more involved jobs and that there are tools around that are much better suited for that. So unless we want to recreate something like Nifi, I think you are probably right in keeping it simple. This would probably encourage the consideration of switching to another tool earlier rather than later, before too much effort has been sunk into an implementation. Summing it up, I am happy for you to proceed in any way you deem useful and I'll stay out of your hair :) Best regards, Sönke On Wed, 1 Apr 2020 at 18:00, Tom Bentley <tbent...@redhat.com> wrote: > Hi Sönke, > > Thanks for taking a look. > > Let me answer in reverse order, since I think it might make more sense that > way... > > Also, while writing that, I noticed that you have a version with and > > without "name" for your transformation in the KIP: > > > > transforms.conditionalExtract.condition.hasMyHeader: has-header:my-header > > and > > transforms.conditionalExtract.condition: has-header:my-header > > > > The example > transforms.conditionalExtract.condition: has-header:my-header > is a "has-header" condition (the prefix of the config value), which will > match records with a "my-header" header (given in the suffix). > > The other example given is: > transforms.conditionalExtract.condition: not:hasMyHeader > transforms.conditionalExtract.condition.hasMyHeader: > has-header:my-header > The root of the condition is a "not" condition (the prefix of the value for > the transforms.conditionalExtract.condition key) of another named condition > called "hasMyHeader" (the suffix). Any name could be used for the other > condition. That other condition is configured at > "transforms.conditionalExtract.condition.<conditionName>". That condition > is a "has-header" condition (the prefix), which will match records with a > "my-header" header (given in the suffix). So the "has-header:" condition > type would always require a suffix, as would the "not:" condition type. > Hypothetically you could have a "true" condition type (which would not > require a suffix), and the hypothetical binary conditions "and:" and "or:" > would require a pair of other condition names. > > So what's proposed is a scheme for encoding conditions where the condition > type is the prefix of the value of some "....condition" config key, and the > optional suffix provides parameters for the condition. This makes those > parameters a bit inflexible, but is relatively succinct. > > This leads on to your first point. You're right that use cases might appear > which need other conditions, and we should make it flexible enough to be > able to cope with future use cases. On the other hand, I was concerned that > we end up with something which is quite complicated to configure. (There > comes a point where it might makes more sense for the user to write their > own SMT). > > Just of the top of my head it might look like: > > > > transforms.conditionalExtract.condition.hasMyHeader: type:has-header > > transforms.conditionalExtract.condition.hasMyHeader: > header-name:my-header > > transforms.conditionalExtract.condition.hasMyHeader: field-value:my-value > > > > That won't work because the format is basically a Properties/Map<String, > String> and what you've suggested has duplicate keys. > > One thing I did briefly consider what the ability to treat conditions as > Configurable objects in their own right (opening up the possibility of > people supplying their own Conditions, just like they can supply their own > SMTs). That might be configured something like this: > > transforms.conditionalExtract.condition: not > transforms.conditionalExtract.condition.not.type: Not > transforms.conditionalExtract.condition.not.negated: foo > transforms.conditionalExtract.condition.foo.type: HasHeaderWithValue > transforms.conditionalExtract.condition.foo.header: my-header > transforms.conditionalExtract.condition.foo.value: my-value > > I didn't propose that I couldn't see the use cases to justify this kind of > complexity, especially as the common case would surely be matching against > topic name (to be honest I wasn't completely convinced by the need for > "has-header"). In the current version of the KIP that's just > > transforms.conditionalExtract.condition: topic-matches: my-prefix-.* > > but using the more flexible scheme that would require something more like > this: > > transforms.conditionalExtract.condition: bar > transforms.conditionalExtract.condition.bar.type: TopicMatch > transforms.conditionalExtract.condition.bar.pattern: my-prefix-.* > > If people know of use cases which would justify more sophistication, I'm > happy to reconsider. > > Thanks again for taking a look! > > Tom > > On Wed, Apr 1, 2020 at 2:05 PM Sönke Liebau > <soenke.lie...@opencore.com.invalid> wrote: > > > Hi Tom, > > > > sounds useful to me, thanks for the KIP! > > The only thought that I had while reading was that this will probably > raise > > questions about more involved conditions fairly quickly. For example the > > "has-header" will cause an appetite for conditions like > > "this-header-has-that-value". > > This would necessitate two parameters to be passed into the condition, > > which I think is not currently included in the KIP. I am not saying add > > this now, but might it make sense to discuss a concept of how that might > > look now, to avoid potential changes later on. > > > > Just of the top of my head it might look like: > > > > transforms.conditionalExtract.condition.hasMyHeader: type:has-header > > transforms.conditionalExtract.condition.hasMyHeader: > header-name:my-header > > transforms.conditionalExtract.condition.hasMyHeader: field-value:my-value > > > > > > Also, while writing that, I noticed that you have a version with and > > without "name" for your transformation in the KIP: > > > > transforms.conditionalExtract.condition.hasMyHeader: has-header:my-header > > and > > transforms.conditionalExtract.condition: has-header:my-header > > > > > > Is this intentional and the name is optional? > > > > > > Best regards, > > > > Sönke > > > > > > > > On Wed, 1 Apr 2020 at 11:12, Tom Bentley <tbent...@redhat.com> wrote: > > > > > > Does anyone have any comments, feedback or thoughts about this? > > > > > > Thanks, > > > > > > Tom > > > > > > On Tue, Mar 24, 2020 at 11:56 AM Tom Bentley <tbent...@redhat.com> > > wrote: > > > > > > > Hi, > > > > > > > > I've opened KIP-585 which is intended to provide a mechanism to > > > > conditionally apply SMTs in Kafka Connect. I'd be grateful for any > > > > feedback: > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-585%3A+Conditional+SMT > > > > > > > > Many thanks, > > > > > > > > Tom > > > > > > >
