Thanks, Sasaki. Colin
On Sat, Apr 28, 2018, at 00:55, Sasaki Toru wrote: > Hi Colin, Jason, > > Thank you for your beneficial comment. > I have updated my Pull Request to show git commit hash in version > information.> In my current Pull Request, we cat get the result such below: > > $ bin/kafka-topics.sh --version > (snip) > 2.0.0-SNAPSHOT (Commit:f3876cd9617faf7e) > > > I have also updated to accept double-dash for this option (-- > version) only.> > > Many thanks, > Sasaki > > > From: Jason Gustafson <ja...@confluent.io> > > Date: 2018-04-25 9:42 GMT+09:00 > > Subject: Re: [DISCUSS] KIP-278: Add version option to Kafka's > > commands> > To: dev <dev@kafka.apache.org> > > > > > > +1 on adding the git commit id to the output. We often encounter > > environments which are testing off of trunk or have modifications on > > top of> > an existing release. > > > > -Jason > > > > On Tue, Apr 24, 2018 at 10:06 AM, Colin McCabe <cmcc...@apache.org> > > wrote:> > > >> On Tue, Apr 24, 2018, at 05:36, Sasaki Toru wrote: > >>> Hi Jason, Colin, > >>> > >>> Thank you for your comment, and I'm sorry for late reply. > >>> > >>> > we refactored all of the tools so that we could use a common > >>> > set of> >>> base options, > >>> > it might be a little annoying to have to continue supporting > >>> > both> >>> variations. > >>> > >>> I feel KIP-14 is good idea (I'm sorry, I didn't know about > >>> it), and> >>> I think it's no longer necessary both single-dash and > >>> double- > >>> dash are> >>> supported when this KIP will be accepted, as you said. > >>> I revise my Pull Request to support single-dash option only. > >>> > >>> Some my code in kafka-run-class.sh will become unnecessary when > >>> KIP-14> >>> is accepted. > >>> But I will keep this as temporary, because I seem that it's useful> >>> > >>> before KIP-14 accepted. > >>> > >>> > >>> > Can you give a little more detail about what would be > >>> > displayed when> >>> the version command was used? > >>> > >>> As Ismael said, the version string is got from > >>> AppInfoParser#getVersion.> >>> > >>> In my Pull Request, we can get the result such as below:: > >>> > >>> $ bin/kafka-topics.sh --version > >>> (snip) > >>> Kafka 1.2.0-SNAPSHOT > >> Hi Sasaki, > >> > >> Thanks for the info. Can you add this to the KIP? > >> > >> Also, I really think we should include the git hash. > >> > >> best, > >> Colin > >> > >> > >>> > >>> Many thanks, > >>> Sasaki > >>> > >>> > >>>> From: Ismael Juma <ism...@juma.me.uk> > >>>> Date: 2018-04-24 3:44 GMT+09:00 > >>>> Subject: Re: [DISCUSS] KIP-278: Add version option to Kafka's > >>>> commands> >>>> To: dev <dev@kafka.apache.org> > >>>> > >>>> > >>>> FYI, the injection via the build process that is mentioned here > > already > >>>> happens. See AppInfoParser. > >>>> > >>>> Ismael > >>>> > >>>> On Mon, Apr 23, 2018 at 9:39 AM, Colin McCabe > >>>> <cmcc...@apache.org>> >> wrote: > >>>>> Hi Sasaki, > >>>>> > >>>>> Thanks for the KIP. I think a version flag is a good idea. > >>>>> > >>>>> Can you give a little more detail about what would be displayed > >>>>> when> >> the > >>>>> version command was used? > >>>>> > >>>>> We clearly want the version number, but we probably also want to > >>>>> know> >> if > >>>>> this is an official release, or a random SNAPSHOT from a branch. > >>>>> If> >> this > >>>>> is a release candidate, we probably want the RC number as well, > >>>>> like> >>>>> "1.1-rc3" We also want a git hash. This can be injected > >>>>> by the> > build > >>>>> process. In the case of an official release, where the source > >>>>> code> >> is not > >>>>> under git, we can pull it from a file. > >>>>> > >>>>> For example, hadoop's version output looks like this: > >>>>> > >>>>> > cmccabe@aurora:~/Downloads/hadoop-2.8.3> ./bin/hadoop > >>>>> > version> >>>>> > Hadoop 2.8.3 > >>>>> > Subversion > >>>>> > https://git-wip-us.apache.org/repos/asf/hadoop.git -r> >>>>> > >>>>> b3fe56402d908019d99af1f1f4fc65cb1d1436a2 > >>>>> > Compiled by jdu on 2017-12-05T03:43Z > >>>>> > Compiled with protoc 2.5.0 > >>>>> > From source with checksum 9ff4856d824e983fa510d3f843e3f19d> >>>>> > >>>>> > This command was run using /home/cmccabe/Downloads/ > >>>>> hadoop-2.8.3/share/hadoop/common/hadoop-common-2.8.3.jar > >>>>> > >>>>> (The "subversion" line here is a little weird -- it now refers > >>>>> to> >> git, not > >>>>> svn) > >>>>> > >>>>> On Wed, Apr 11, 2018, at 13:58, Jason Gustafson wrote: > >>>>>> Hey Sasaki, > >>>>>> > >>>>>> Yeah, I don't feel too strongly about only supporting -- > >>>>>> version. I> >> agree > >>>>> it > >>>>>> may help discoverability given the current approach. On the > >>>>>> other> >> hand, > >>>>> if > >>>>>> we refactored all of the tools so that we could use a common > >>>>>> set of> >> base > >>>>>> options, it might be a little annoying to have to continue > > supporting > >>>>> both > >>>>>> variations. For example, tool standardization was proposed in > >>>>>> KIP-14> >> and > >>>>>> I'm still holding out hope that someone will have time to pick > >>>>>> this> >> work > >>>>>> back up. It's always easier to add an option than remove one, > >>>>>> so I'm> >>>>>> slightly inclined to have only --version for now. What > >>>>>> do you > >>>>>> think?> >>>>> The double dash version is more consistent with how our > >>>>>> other > >>>>> flags> >> work. > >>>>> In general, I feel that if --version is supported, --help should > >>>>> say> >> so. > >>>>> best, > >>>>> Colin > >>>>> > >>>>> > >>>>>> Thanks, > >>>>>> Jason > >>>>>> > >>>>>> On Tue, Apr 10, 2018 at 12:00 AM, Sasaki Toru < > >> sasaki...@oss.nttdata.com > >>>>>> wrote: > >>>>>> > >>>>>>> Hi Jason > >>>>>>> > >>>>>>> Thank you for helpful comments. I updated wiki based on your > > advice. > >>>>>>> I thought this option was relatively common and making > >>>>>>> maintenance> >>>> easy > >>>>>>> was also important. > >>>>>>> However, as you said, it is not good that version option won't > >>>>>>> be> >>>>> shown up > >>>>>>> in help description. > >>>>>>> > >>>>>>> I thought accepting both single-dash and double-dash will help > >>>>>>> to> >> find > >>>>>>> this option. > >>>>>>> In my approach this option won't be showed, but most of > >>>>>>> software> >> which > >>>>> has > >>>>>>> this option accepts either single-dash or double-dash. > >>>>>>> I guess it doesn't need to support both if we take another > >>>>>>> way.> >>>>>>> > >>>>>>> > >>>>>>> Thanks > >>>>>>> > >>>>>>> @Ted Yeah, you're right. Sorry about the confusion. > >>>>>>>> Since we're here, I think this KIP is a nice improvement. > >>>>>>>> It's> >>>>> definitely > >>>>>>>> nice to have an easy way to check the version. That said, do > >>>>>>>> we> >>>> really > >>>>>>>> need > >>>>>>>> to support both `-version` and `--version`? The latter is > >> consistent > >>>>> with > >>>>>>>> our current tools. > >>>>>>>> > >>>>>>>> Also, I think the approach we've taken is basically to build > >>>>>>>> the> >>>>> --version > >>>>>>>> functionality into the bash script. This is nice because it > >>>>>>>> saves> > a > >>>>> lot of > >>>>>>>> work to update the commands individually and we don't need to > >>>>>>>> do> >>>>> anything > >>>>>>>> when we add new tools. The downside is that `--version` won't > >>>>>>>> show> >> up > >>>>> as > >>>>>>>> an > >>>>>>>> option in any of the --help output. Not sure if that is too > >>>>>>>> big of> >> a > >>>>>>>> problem, but maybe worth mentioning this in the rejected > >> alternatives > >>>>>>>> section. > >>>>>>>> > >>>>>>>> > >>>>>>>> -Jason > >>>>>>>> > >>>>>>>> On Wed, Apr 4, 2018 at 9:42 AM, Ted Yu <yuzhih...@gmail.com>> >> > >>>>>>>> wrote: > >>>>>>>> Jason: > >>>>>>>>> Maybe your reply was intended for another KIP ? > >>>>>>>>> > >>>>>>>>> KIP-278 is about adding version option, not timeout. > >>>>>>>>> > >>>>>>>>> Cheers > >>>>>>>>> > >>>>>>>>> On Wed, Apr 4, 2018 at 9:36 AM, Jason Gustafson < > >> ja...@confluent.io> > >>>>>>>>> wrote: > >>>>>>>>> > >>>>>>>>> Hi Sasaki, > >>>>>>>>>> Thanks for the KIP. I think the timeout controls the > >>>>>>>>>> maximum> >>>> allowed > >>>>>>>>> time > >>>>>>>>> that the consumer will block for the next record. Maybe the> >> > >>>>>>>>> meaning > >>>>>>>>> would > >>>>>>>>> be clearer with the more concise name `--timeout`? That also > >>>>>>>>> fits> >>>>> with > >>>>>>>>> the > >>>>>>>>> > >>>>>>>>>> old consumer which overrides the `consumer.timeout.ms` > >>>>>>>>>> property.> >>>>>>>>>> > >>>>>>>>>> By the way, it seems like the default value was > >>>>>>>>>> intentionally> > set > >>>>> low > >>>>>>>>> for > >>>>>>>>> both the old and new consumers, but I'm not sure of the > >>>>>>>>> reason.> > We > >>>>> could > >>>>>>>>>> leave the default as it is if we want to be safe, but > >>>>>>>>>> increasing> >> it > >>>>>>>>> seems > >>>>>>>>> ok to me. Perhaps we could start a little lower, though, say > >>>>>>>>> 10> >>>>> seconds? > >>>>>>>>> In > >>>>>>>>> > >>>>>>>>>> any case, we should make it clear to the user that the > >>>>>>>>>> timeout> >> was > >>>>>>>>> reached. > >>>>>>>>> > >>>>>>>>>> It's surprising to see only the incomplete reported results> >>>>> > >>>>>>>>>> following a > >>>>>>>>>> timeout. > >>>>>>>>>> > >>>>>>>>>> Thanks, > >>>>>>>>>> Jason > >>>>>>>>>> > >>>>>>>>>> On Wed, Apr 4, 2018 at 4:37 AM, Sasaki Toru < > >>>>> sasaki...@oss.nttdata.com> > >>>>>>>>>> wrote: > >>>>>>>>>> > >>>>>>>>>> Hello everyone, > >>>>>>>>>>> I would like to start a discussion for KIP 278. Cloud you> > > >>>>>>>>>>> please > >>>>> give > >>>>>>>>>>> comments and advice ? > >>>>>>>>>>> <https://cwiki.apache.org/confluence/display/KAFKA/KIP- > >>>>>>>>>>> 278+-> >>>>>>>>>>> +Add+version+option+to+Kafka%27s+commands> > >>>>>>>>>>> > >>>>>>>>>>> JIRA ticket and Pull Request are bellow: > >>>>>>>>>>> <https://issues.apache.org/jira/browse/KAFKA-2061> > >>>>>>>>>>> <https://github.com/apache/kafka/pull/639> > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> Many thanks, > >>>>>>>>>>> > >>>>>>>>>>> Sasaki > >>>>>>>>>>> > >>>>>>>>>>> -- > >>>>>>>>>>> Sasaki Toru(sasaki...@oss.nttdata.com) NTT DATA > >>>>>>>>>>> CORPORATION> >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>> -- > >>>>>>> Sasaki Toru(sasaki...@oss.nttdata.com) NTT DATA CORPORATION > >>>>>>> > >>>>>>> > >>> -- > >>> Sasaki Toru(sasaki...@oss.nttdata.com) NTT DATA CORPORATION > >>> > > -- > Sasaki Toru(sasaki...@oss.nttdata.com) NTT DATA CORPORATION >
