You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Shikhar Bhushan <sh...@confluent.io> on 2016/06/23 18:06:09 UTC
[DISCUSS] KIP-65 Expose timestamps to Connect
Kafkarati,
Here is a pretty straightforward proposal, for exposing timestamps that
were added in Kafka 0.10 to the connect framework so connectors can make
use of them:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-65%3A+Expose+timestamps+to+Connect
Appreciate your thoughts!
Shikhar
Re: [DISCUSS] KIP-65 Expose timestamps to Connect
Posted by Shikhar Bhushan <sh...@confluent.io>.
I updated the KIP and PR with this change
On Fri, Jun 24, 2016 at 4:58 PM Ismael Juma <is...@juma.me.uk> wrote:
> Yes, I agree that it would be better to be consistent. I suggest `Long` and
> `null` everywhere if feasible as it's less opaque than the magic -1L value.
> The KIP page should be updated with what you decide.
>
> Ismael
>
> On Sat, Jun 25, 2016 at 1:29 AM, Shikhar Bhushan <sh...@confluent.io>
> wrote:
>
> > Hi Ismael,
> >
> > Good point. This is down to an implementation detail, the getter was
> added
> > to the base class for `SourceRecord` and `SinkRecord`,
> > `ConnectRecord`. `SourceRecord`
> > is treating missing timestamps as null while `SinkRecord` is treating it
> as
> > the default value `Record.NO_TIMESTAMP` (-1L).
> >
> > It probably makes sense to be consistent and use either Long everywhere
> or
> > the primitive long and default values.
> >
> > Feel free to add the comment on the PR
> > <https://github.com/apache/kafka/pull/1537/files> as well and I can
> follow
> > up there :-)
> >
> > Thanks,
> >
> > Shikhar
> >
> > On Fri, Jun 24, 2016 at 3:52 PM Ismael Juma <is...@juma.me.uk> wrote:
> >
> > > Hi Shikhar,
> > >
> > > Thanks for the KIP. One question:
> > >
> > > SinkRecord takes a `long` timestamp, but then exposes it via a method
> > that
> > > returns `Long`. Is this correct? And if so, can you please explain the
> > > reasoning?
> > >
> > > Ismael
> > >
> > > On Thu, Jun 23, 2016 at 8:06 PM, Shikhar Bhushan <shikhar@confluent.io
> >
> > > wrote:
> > >
> > > > Kafkarati,
> > > >
> > > > Here is a pretty straightforward proposal, for exposing timestamps
> that
> > > > were added in Kafka 0.10 to the connect framework so connectors can
> > make
> > > > use of them:
> > > >
> > > >
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-65%3A+Expose+timestamps+to+Connect
> > > >
> > > > Appreciate your thoughts!
> > > >
> > > > Shikhar
> > > >
> > >
> >
>
Re: [DISCUSS] KIP-65 Expose timestamps to Connect
Posted by Ismael Juma <is...@juma.me.uk>.
Yes, I agree that it would be better to be consistent. I suggest `Long` and
`null` everywhere if feasible as it's less opaque than the magic -1L value.
The KIP page should be updated with what you decide.
Ismael
On Sat, Jun 25, 2016 at 1:29 AM, Shikhar Bhushan <sh...@confluent.io>
wrote:
> Hi Ismael,
>
> Good point. This is down to an implementation detail, the getter was added
> to the base class for `SourceRecord` and `SinkRecord`,
> `ConnectRecord`. `SourceRecord`
> is treating missing timestamps as null while `SinkRecord` is treating it as
> the default value `Record.NO_TIMESTAMP` (-1L).
>
> It probably makes sense to be consistent and use either Long everywhere or
> the primitive long and default values.
>
> Feel free to add the comment on the PR
> <https://github.com/apache/kafka/pull/1537/files> as well and I can follow
> up there :-)
>
> Thanks,
>
> Shikhar
>
> On Fri, Jun 24, 2016 at 3:52 PM Ismael Juma <is...@juma.me.uk> wrote:
>
> > Hi Shikhar,
> >
> > Thanks for the KIP. One question:
> >
> > SinkRecord takes a `long` timestamp, but then exposes it via a method
> that
> > returns `Long`. Is this correct? And if so, can you please explain the
> > reasoning?
> >
> > Ismael
> >
> > On Thu, Jun 23, 2016 at 8:06 PM, Shikhar Bhushan <sh...@confluent.io>
> > wrote:
> >
> > > Kafkarati,
> > >
> > > Here is a pretty straightforward proposal, for exposing timestamps that
> > > were added in Kafka 0.10 to the connect framework so connectors can
> make
> > > use of them:
> > >
> > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-65%3A+Expose+timestamps+to+Connect
> > >
> > > Appreciate your thoughts!
> > >
> > > Shikhar
> > >
> >
>
Re: [DISCUSS] KIP-65 Expose timestamps to Connect
Posted by Shikhar Bhushan <sh...@confluent.io>.
Hi Ismael,
Good point. This is down to an implementation detail, the getter was added
to the base class for `SourceRecord` and `SinkRecord`,
`ConnectRecord`. `SourceRecord`
is treating missing timestamps as null while `SinkRecord` is treating it as
the default value `Record.NO_TIMESTAMP` (-1L).
It probably makes sense to be consistent and use either Long everywhere or
the primitive long and default values.
Feel free to add the comment on the PR
<https://github.com/apache/kafka/pull/1537/files> as well and I can follow
up there :-)
Thanks,
Shikhar
On Fri, Jun 24, 2016 at 3:52 PM Ismael Juma <is...@juma.me.uk> wrote:
> Hi Shikhar,
>
> Thanks for the KIP. One question:
>
> SinkRecord takes a `long` timestamp, but then exposes it via a method that
> returns `Long`. Is this correct? And if so, can you please explain the
> reasoning?
>
> Ismael
>
> On Thu, Jun 23, 2016 at 8:06 PM, Shikhar Bhushan <sh...@confluent.io>
> wrote:
>
> > Kafkarati,
> >
> > Here is a pretty straightforward proposal, for exposing timestamps that
> > were added in Kafka 0.10 to the connect framework so connectors can make
> > use of them:
> >
> >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-65%3A+Expose+timestamps+to+Connect
> >
> > Appreciate your thoughts!
> >
> > Shikhar
> >
>
Re: [DISCUSS] KIP-65 Expose timestamps to Connect
Posted by Ismael Juma <is...@juma.me.uk>.
Hi Shikhar,
Thanks for the KIP. One question:
SinkRecord takes a `long` timestamp, but then exposes it via a method that
returns `Long`. Is this correct? And if so, can you please explain the
reasoning?
Ismael
On Thu, Jun 23, 2016 at 8:06 PM, Shikhar Bhushan <sh...@confluent.io>
wrote:
> Kafkarati,
>
> Here is a pretty straightforward proposal, for exposing timestamps that
> were added in Kafka 0.10 to the connect framework so connectors can make
> use of them:
>
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-65%3A+Expose+timestamps+to+Connect
>
> Appreciate your thoughts!
>
> Shikhar
>