Re: [DISCUSS] KIP-131 : Add access to OffsetStorageReader from SourceConnector

[Randall Hauch <rh...@gmail.com>]

Thanks for taking into account my suggestions/concerns. I had a few very
minor suggestions on the PR regarding documentation, but overall everything
looks great to me.

I'd encourage anyone else to review
https://cwiki.apache.org/confluence/display/KAFKA/KIP-131+-+Add+access+to+OffsetStorageReader+from+SourceConnector
and the PR, and to then provide feedback here. If we hear nothing, we'll
submit for a vote.

Best regards,

Randall

On Thu, Aug 24, 2017 at 12:38 PM, Florian Hussonnois <fh...@gmail.com>
wrote:

> Hi Randall,
>
> Thank you for your answer.
>
> I will update the KIP and the PR with your last approach which sounds
> better.
>
> Thanks.
>
> Le 16 août 2017 00:53, "Randall Hauch" <rh...@gmail.com> a écrit :
>
> Sorry it's taken me so long to come back to this.
>
> Have you considered creating a `SourceConnectorContext` interface that
> extends `ConnectorContext` and that adds the method to access the offset
> storage? This would very closely match the existing `SourceTaskContext`.
>
> `SourceConnector` implementations could always cast the `context` field in
> the superclass to `SourceConnectorContext`, but perhaps a slightly better
> way to do this is to add the following method to the `SourceConnector`
> class:
>
>
>     public SourceConnectorContext context() {
>         return (SourceConnectorContext)context;
>     }
>
>
> Now, `SourceConnector` implementations can either cast themselves or use
> this additional method to obtain the correctly cast context.
>
> In fact, it might be good to do this for `SinkConnector` as well, and we
> could even add a `context()` method in the `Connector` interface, since
> subinterfaces can change the return type to be a subtype of that returned
> by the interface:
>
>     ConnectorContext context();
>
> One advantage of this approach is that `SourceConnectorContext` and
> `SinkConnectorContext` remain interfaces. Another is not adding new method
> to `SourceConnector` that implementers may need to learn that they should
> not override or implement them. A third is that now we have a
> `SourceConnectorContext` and `SinkConnectorContext` to which we can add
> more methods if needed, and they are very similar to `SourceTaskContext`
> and `SinkTaskContext`.
>
> Thoughts?
>
> On Wed, Apr 5, 2017 at 3:59 PM, Florian Hussonnois <fh...@gmail.com>
> wrote:
>
> > Hi All,
> >
> > Is there any feedback regarding that KIP ?
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 131+-+Add+access+to+
> > OffsetStorageReader+from+SourceConnector
> >
> > Thanks,
> >
> > 2017-03-14 22:51 GMT+01:00 Florian Hussonnois <fh...@gmail.com>:
> >
> > > Hi Matthias,
> > >
> > > Sorry I didn't know this page. Ths KIP has been added to it.
> > >
> > > Thanks,
> > >
> > > 2017-03-13 21:30 GMT+01:00 Matthias J. Sax <ma...@confluent.io>:
> > >
> > >> Can you please add the KIP to this table:
> > >>
> > >> https://cwiki.apache.org/confluence/display/KAFKA/Kafka+
> > >> Improvement+Proposals#KafkaImprovementProposals-KIPsunderdiscussion
> > >>
> > >> Thanks,
> > >>
> > >>  Matthias
> > >>
> > >>
> > >> On 3/7/17 1:24 PM, Florian Hussonnois wrote:
> > >> > Hi all,
> > >> >
> > >> > I've created a new KIP to add access to OffsetStorageReader from
> > >> > SourceConnector
> > >> >
> > >> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-131+-+
> > >> Add+access+to+OffsetStorageReader+from+SourceConnector
> > >> >
> > >> > Thanks.
> > >> >
> > >>
> > >>
> > >
> > >
> > > --
> > > Florian HUSSONNOIS
> > >
> >
> >
> >
> > --
> > Florian HUSSONNOIS
> >
>