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
