You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@kafka.apache.org by Chia-Ping Tsai <ch...@apache.org> on 2018/07/31 08:10:03 UTC

[DISCUSS] KIP-348 Eliminate null from SourceTask#poll()

hi all,

Please take a look at the KIP-348[1] if you have free cycel. It bring a little change to the usage of SourceTask#poll()


[1] https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=89065853&moved=true#KIP-348EliminatenullfromSourceTask#poll()-Status

Cheers,
Chia-Ping

Re: [DISCUSS] KIP-348 Eliminate null from SourceTask#poll()

Posted by Colin McCabe <cm...@apache.org>.

Thanks, Chia-Ping.  Looks good to me.

regards,
Colin

On Sat, Aug 4, 2018, at 01:17, Chia-Ping Tsai wrote:
> hi Colin
> 
> Thanks for the reviews! You are totally right. The description of 
> KIP-348 is not accurate. The purpose of KIP-348 is to encourage 
> connector user to substitute empty list to null value, but at the same 
> time returning null still work.
> 
> I will update the KIP-348 ASAP.
> 
> cheers,
> Chia-Ping
> 
> On 2018/08/03 22:56:34, Colin McCabe <cm...@apache.org> wrote: 
> > "No changes to public interface" doesn't seem accurate here.  SourceTask#poll is a public interface, right?  Kafka connectors that are out-of-tree would certainly break if we disallowed returning null from this method.
> > 
> > However, reading the KIP more closely, it seems like both null and the empty list will be supported.  Perhaps you should discuss this in the compatibility section?  Also, the KIP should probably be renamed "deprecate null" rather than "eliminate null" since it will still be possible to return null here, right?
> > 
> > best,
> > Colin
> > 
> > 
> > On Tue, Jul 31, 2018, at 01:10, Chia-Ping Tsai wrote:
> > > hi all,
> > > 
> > > Please take a look at the KIP-348[1] if you have free cycel. It bring a 
> > > little change to the usage of SourceTask#poll()
> > > 
> > > 
> > > [1] 
> > > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=89065853&moved=true#KIP-348EliminatenullfromSourceTask#poll()-
> > > Status
> > > 
> > > Cheers,
> > > Chia-Ping
> >

Re: [DISCUSS] KIP-348 Eliminate null from SourceTask#poll()

Posted by Chia-Ping Tsai <ch...@apache.org>.

hi Colin

Thanks for the reviews! You are totally right. The description of KIP-348 is not accurate. The purpose of KIP-348 is to encourage connector user to substitute empty list to null value, but at the same time returning null still work.

I will update the KIP-348 ASAP.

cheers,
Chia-Ping

On 2018/08/03 22:56:34, Colin McCabe <cm...@apache.org> wrote: 
> "No changes to public interface" doesn't seem accurate here.  SourceTask#poll is a public interface, right?  Kafka connectors that are out-of-tree would certainly break if we disallowed returning null from this method.
> 
> However, reading the KIP more closely, it seems like both null and the empty list will be supported.  Perhaps you should discuss this in the compatibility section?  Also, the KIP should probably be renamed "deprecate null" rather than "eliminate null" since it will still be possible to return null here, right?
> 
> best,
> Colin
> 
> 
> On Tue, Jul 31, 2018, at 01:10, Chia-Ping Tsai wrote:
> > hi all,
> > 
> > Please take a look at the KIP-348[1] if you have free cycel. It bring a 
> > little change to the usage of SourceTask#poll()
> > 
> > 
> > [1] 
> > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=89065853&moved=true#KIP-348EliminatenullfromSourceTask#poll()-
> > Status
> > 
> > Cheers,
> > Chia-Ping
>

Re: [DISCUSS] KIP-348 Eliminate null from SourceTask#poll()

Posted by Colin McCabe <cm...@apache.org>.

"No changes to public interface" doesn't seem accurate here.  SourceTask#poll is a public interface, right?  Kafka connectors that are out-of-tree would certainly break if we disallowed returning null from this method.

However, reading the KIP more closely, it seems like both null and the empty list will be supported.  Perhaps you should discuss this in the compatibility section?  Also, the KIP should probably be renamed "deprecate null" rather than "eliminate null" since it will still be possible to return null here, right?

best,
Colin

On Tue, Jul 31, 2018, at 01:10, Chia-Ping Tsai wrote:
> hi all,
> 
> Please take a look at the KIP-348[1] if you have free cycel. It bring a 
> little change to the usage of SourceTask#poll()
> 
> 
> [1] 
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=89065853&moved=true#KIP-348EliminatenullfromSourceTask#poll()-
> Status
> 
> Cheers,
> Chia-Ping