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 > > > >
