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 <fhussonn...@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" <rha...@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 <fhussonn...@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 <fhussonn...@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 <matth...@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 > > >
