Hi there,
Bumping this thread for visibility.
Cheers,
Jorge
On Fri, 2 Sep 2022, 18:01 Chris Egerton, wrote:
> Hi Jorge,
>
> One tiny nit, but LGTM otherwise:
>
> The KIP mentions backslashes as "(/)"; shouldn't this be "(\)"?
>
> I'll cast a +1 on the vote thread anyways; I'm sure this won't
Hi Jorge,
One tiny nit, but LGTM otherwise:
The KIP mentions backslashes as "(/)"; shouldn't this be "(\)"?
I'll cast a +1 on the vote thread anyways; I'm sure this won't block us.
Cheers, and thanks for all your hard work on this!
Chris
On Thu, Sep 1, 2022 at 1:33 PM Jorge Esteban Quilcate
Hi Chris,
Thanks for your feedback!
1. Yes, it will be context-dependent. I have added rules and scenarios to
the nested notation to cover the happy path and edge cases. In short,
backticks will be not be considered as part of the field name when they are
wrapping a field name: first backtick at
Hi Robert and Jorge,
I think the backtick/backslash proposal works, but I'm a little unclear on
some of the details:
1. Are backticks only given special treatment when they immediately follow
a non-escaped dot? E.g., "foo.b`ar.ba`z" would refer to "foo" -> "b`ar" ->
"ba`z" instead of "foo" ->
Awesome, thanks Robert!
Will work on updated the proposal around this. Also, @Chris Egerton
if you have any feedback about this, would be
much appreciated.
Cheers,
Jorge.
On Fri, 12 Aug 2022 at 18:45, Robert Yokota wrote:
> Hi Jorge,
>
> Yes, to escape a backtick I would recommend a
Hi Jorge,
Yes, to escape a backtick I would recommend a backslash.
For example, MySQL allows identifiers to be surrounded with single quotes,
double quotes, or backticks, and they use backslash to escape.
Jsonata went with primarily backticks as they are less common in strings
that appear in
Thanks Robert! And sorry for the late reply.
That's a great catch and will require the current proposal to be updated.
Using back-ticks is a good proposal. Though, the challenge we got is that
JSON allows really any character into attribute names, including back-ticks.
I wonder what's the best
Hi,
I'm late to this thread, but would like to comment on the double dot/double
asterisk syntax.
Unfortunately double dot is often used in JSON Path as a descendant
selector, see https://www.ietf.org/id/draft-ietf-jsonpath-base-05.html
I think a better notation would be to use backticks to
Thanks Chris! I have updated the KIP to include this fix.
I will keep the array as a potential improvement at the moment, and out of
the scope of this KIP.
Thanks,
Jorge.
On Tue, 28 Jun 2022 at 23:19, Chris Egerton wrote:
> Hi Jorge,
>
> Apologies for the long delay, had my own KIP-related
Hi Jorge,
Apologies for the long delay, had my own KIP-related work to focus on.
I think it's fine to include array accesses but it's not a blocker. I'm +1
either way. On that front though, I think the MaskField section might need
to be updated as it still mentions arrays and deep-scan?
Cheers,
Hi there,
I have update the KIP to the previous state voted, including the
configuration change from `field.style` to `field.syntax.version`.
I'll bump the vote thread as well to see if there's agreement on adding
this feature to Connect.
Cheers,
Jorge.
On Wed, 15 Jun 2022 at 23:02, Jorge
Thanks, Chris. Your feedback is much appreciated!
I see how the current proposal might be underestimating some edge cases.
I'm happy to move the design for deep-scan and multi-values to future
developments related with this KIP and reduce its scope, though open for
more feedback.
Also, just to
Hi Jorge,
I've done some more thinking and I hate to say it, but I think the syntax
does need to be expanded. Right now it's clear what "a.b" refers to and
what "a..b" refers to, but what about "a...b"? Is that referring to
subfield ".b" of field "a", or subfield "b" of field "a."? This gets even
Thanks, Chris!
Please, find my comments below:
On Tue, 7 Jun 2022 at 04:39, Chris Egerton wrote:
> Hi Jorge,
>
> Thanks! Sorry for the delay; here are my thoughts:
>
> 1. Under the "Accessing multiple values by deep-scan" header it's stated
> that "If deep-scan is used, it must have only one
Hi Jorge,
Thanks! Sorry for the delay; here are my thoughts:
1. Under the "Accessing multiple values by deep-scan" header it's stated
that "If deep-scan is used, it must have only one field after the asterisk
level.". However, in example 3 for the Cast SMT and other examples for
other SMTs, the
Thanks, Chris!
I have updated the KIP with the rejected alternatives updated. Also, I have
drafted the support for arrays and deep scans as part of the proposed
notation to make it more complete, even though these can be implemented in
multiple PRs.
Looking forward to your feedback.
Cheers,
Hi Jorge,
I really appreciate the effort you've made to simplify the syntax and
feature set of a JSONPath-based approach as much as possible. I'm still
hesitant to continue with it, though.
1. The syntax is much less friendly. Just compare "top.mid.bottom" to
"$['top']['mid']['bottom']"... not
Thank you all for your feedback, and sorry for the long wait for a reply.
I would like to explore the idea of JSONPath-inspired/subset notation a bit
further:
It will need to be a much-reduced version of JSONPath:
- No full support for JsonPath therefore an additional dependency.
- All paths
Hi Joshua,
I have a few reservations about using JsonPath notation here.
1. There's likely to be a substantial performance penalty for converting
between the Kafka Connect format and something that a JsonPath library
would understand.
2. The complexity of the feature will be significantly
Hello all!
Sorry that I come a bit later to the party here, but I am the one who wrote
KIP-683 [1] for recursive support (just simply looping through all child
non-primitive structures for the same matching name(s)) which is a slightly
different way to try and solve a similar requirement --
Hi Tom,
Thanks for taking a look at this, and for your thoughtful comments. I'll
leave it up to Jorge to address most of your comments but I wanted to share
a couple quick thoughts I had regarding 103 and 104.
103. Like you, I was envisioning a possible syntax for array access that
used classic
Hi Jorge,
Thanks for the KIP, especially for the examples which are super-clear.
100. The name `field.style` isn't so clear for something like ReplaceField:
it's not so obvious that field.style applies to `include` and `exclude`.
101. The permitted values for `field.style` don't seem terribly
Thanks Jorge, LGTM!
On Wed, Apr 20, 2022, 12:40 Jorge Esteban Quilcate Otoya <
quilcate.jo...@gmail.com> wrote:
> Thank you, Chris! Not possible without your feedback.
>
> On Tue, 19 Apr 2022 at 23:04, Chris Egerton
> wrote:
>
> > Hi Jorge,
> >
> > Thank you for sticking through this. I have
Thank you, Chris! Not possible without your feedback.
On Tue, 19 Apr 2022 at 23:04, Chris Egerton wrote:
> Hi Jorge,
>
> Thank you for sticking through this. I have one small remark and one small
> clarification; assuming you agree with me on them then I'm ready to vote on
> the KIP.
>
> 1.
Hi Jorge,
Thank you for sticking through this. I have one small remark and one small
clarification; assuming you agree with me on them then I'm ready to vote on
the KIP.
1. InsertField: The "field.on.missing.parent" and "field.on.existing.field"
docs both mention a permitted value of "ingore";
Thank you, Chris! I apply these improvements to the KIP, let me know how it
looks now.
On Mon, 11 Apr 2022 at 23:43, Chris Egerton wrote:
> Hi Jorge,
>
> Wow, those examples are great! A few more remarks, but I think we're
> getting close:
>
> 1. The examples differ across SMTs with the name of
Hi Jorge,
Wow, those examples are great! A few more remarks, but I think we're
getting close:
1. The examples differ across SMTs with the name of the newly-introduced
style property; some of them use "field.style", and some use
"fields.style". I think for consistency's sake we should stick with
Again, great feedback Chris. Much appreciated.
Added my comments below:
On Tue, 5 Apr 2022 at 20:22, Chris Egerton wrote:
> Hi Jorge,
>
> Looking good! I have a few comments left but all but one or two are minor.
>
> 1. The motivation section states "This KIP is aimed to include support for
>
Hi Jorge,
Looking good! I have a few comments left but all but one or two are minor.
1. The motivation section states "This KIP is aimed to include support for
nested structures on the existing SMTs... and to include the abstractions
to reuse this in future SMTs". A good implementation of this
Thanks, Chris! Much appreciated all the feedback here.
1. You nailed it setting the design goal here: "it shouldn't be impossible
to use this new feature for any field name, no matter how convoluted. It's
fine if edge cases introduce difficulty (such as less-readable
configurations), but it's not
Hi Jorge,
Thanks for addressing my comments; the KIP looks up-to-date and pretty
readable now, and the rejected alternatives section does a great job of
outlining the discussion so far and providing context for anyone else who
might want to join in.
1. Thoughts on choice of delimiter:
- I like
Thanks, Chris!
1. I'd argue "this..field.child" could be harder to grasp than
"this.field/child" + separator: "/".
Even though this represents additional information, it follows a similar
approach as the "Flatten#delimeter" configuration.
I want to give the separator approach another try, so I
Hi Jorge,
Looking good! Got a few more thoughts.
1. Sorry to revisit this, but I think we may want to adopt a slightly
different escape syntax style. Backslashes are great, but since they're
already used by JSON, using them as an escape sequence in field notation
would also lead to some pretty
Thank you, Chris! and sorry for the delayed response.
Please, find my comments below:
On Mon, 14 Feb 2022 at 17:34, Chris Egerton
wrote:
> Hi Jorge,
>
> Thanks for the KIP! I'd love to see support for nested fields added to the
> out-of-the-box SMTs provided with Connect. Here are my initial
Hi Jorge,
Thanks for the KIP! I'd love to see support for nested fields added to the
out-of-the-box SMTs provided with Connect. Here are my initial thoughts:
1. I agree that there's a case to be made for expanding HoistField with a
new config property for identifying a nested, to-be-hoisted
35 matches
Mail list logo
