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