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 >
