You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Hector Acosta (Code Review)" <ge...@cloudera.org> on 2019/03/12 15:53:41 UTC
[Impala-ASF-CR] Use test -x to check for ntp-wait
Hector Acosta has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12726
Change subject: Use test -x to check for ntp-wait
......................................................................
Use test -x to check for ntp-wait
In centos 7, running ntp-wait --help results in:
/usr/sbin/ntp-wait version [unknown] calling Getopt::Std::getopts
(version 1.07 [paranoid]),
running under Perl version 5.16.3.
Usage: ntp-wait [-OPTIONS [-MORE_OPTIONS]] [--] [PROGRAM_ARG1 ...]
The following single-character options are accepted:
With arguments: -n -s
Boolean (without arguments): -v
Options may be merged together. -- stops processing of options.
Space is not required between options and their arguments.
[Now continuing due to backward compatibility and excessive paranoia.
See 'perldoc Getopt::Std' about $Getopt::Std::STANDARD_HELP_VERSION.]
So instead of running it we get the path using command -v.
Change-Id: I53c63dfa651ac242050171da70540d24c4caf32c
---
M testdata/cluster/admin
1 file changed, 10 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/12726/1
--
To view, visit http://gerrit.cloudera.org:8080/12726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I53c63dfa651ac242050171da70540d24c4caf32c
Gerrit-Change-Number: 12726
Gerrit-PatchSet: 1
Gerrit-Owner: Hector Acosta <he...@cloudera.com>
[Impala-ASF-CR] Use test -x to check for ntp-wait
Posted by "Hector Acosta (Code Review)" <ge...@cloudera.org>.
Hector Acosta has posted comments on this change. ( http://gerrit.cloudera.org:8080/12726 )
Change subject: Use test -x to check for ntp-wait
......................................................................
Patch Set 1:
> This looks good to me. Quick question: Is this a cosmetic issue or
> is there an actual failure case? I'm wondering if we want a JIRA
> for this. If it is cosmetic, then I think we can skip the JIRA.
There's a failure case:
ntp-wait runs `/usr/sbin/ntpq -c "rv 0"` regardless of whether --help is used or not.. Its exit status depends on the output of ntpq.
So for example:
[root@8280aa22322d /]# ntp-wait --help ; echo $?
/usr/sbin/ntp-wait version [unknown] calling Getopt::Std::getopts (version 1.07 [paranoid]),
running under Perl version 5.16.3.
Usage: ntp-wait [-OPTIONS [-MORE_OPTIONS]] [--] [PROGRAM_ARG1 ...]
The following single-character options are accepted:
With arguments: -n -s
Boolean (without arguments): -v
Options may be merged together. -- stops processing of options.
Space is not required between options and their arguments.
[Now continuing due to backward compatibility and excessive paranoia.
See 'perldoc Getopt::Std' about $Getopt::Std::STANDARD_HELP_VERSION.]
1
[root@8280aa22322d /]# ntpd
[root@8280aa22322d /]# ntp-wait --help ; echo $?
/usr/sbin/ntp-wait version [unknown] calling Getopt::Std::getopts (version 1.07 [paranoid]),
running under Perl version 5.16.3.
Usage: ntp-wait [-OPTIONS [-MORE_OPTIONS]] [--] [PROGRAM_ARG1 ...]
The following single-character options are accepted:
With arguments: -n -s
Boolean (without arguments): -v
Options may be merged together. -- stops processing of options.
Space is not required between options and their arguments.
[Now continuing due to backward compatibility and excessive paranoia.
See 'perldoc Getopt::Std' about $Getopt::Std::STANDARD_HELP_VERSION.]
0
It also looks like --help is simply ignored.
--
To view, visit http://gerrit.cloudera.org:8080/12726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I53c63dfa651ac242050171da70540d24c4caf32c
Gerrit-Change-Number: 12726
Gerrit-PatchSet: 1
Gerrit-Owner: Hector Acosta <he...@cloudera.com>
Gerrit-Reviewer: Hector Acosta <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Wed, 13 Mar 2019 20:05:36 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8307 Use test -x to check for ntp-wait
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12726 )
Change subject: IMPALA-8307 Use test -x to check for ntp-wait
......................................................................
IMPALA-8307 Use test -x to check for ntp-wait
Running ntp-wait --help can return 0 or 1 depending on the
output of ntpq. For example:
$ sudo ntp-wait --help 2> /dev/null; echo $?
0
$ sudo killall ntpd
$ sudo ntp-wait --help 2> /dev/null; echo $?
1
This commit instead tests whether ntp-wait exists and is executable to
determine if it's installed.
Change-Id: I53c63dfa651ac242050171da70540d24c4caf32c
Reviewed-on: http://gerrit.cloudera.org:8080/12726
Reviewed-by: Joe McDonnell <jo...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M testdata/cluster/admin
1 file changed, 10 insertions(+), 7 deletions(-)
Approvals:
Joe McDonnell: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/12726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I53c63dfa651ac242050171da70540d24c4caf32c
Gerrit-Change-Number: 12726
Gerrit-PatchSet: 3
Gerrit-Owner: Hector Acosta <he...@cloudera.com>
Gerrit-Reviewer: Hector Acosta <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
[Impala-ASF-CR] IMPALA-8307 Use test -x to check for ntp-wait
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12726 )
Change subject: IMPALA-8307 Use test -x to check for ntp-wait
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2443/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/12726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I53c63dfa651ac242050171da70540d24c4caf32c
Gerrit-Change-Number: 12726
Gerrit-PatchSet: 2
Gerrit-Owner: Hector Acosta <he...@cloudera.com>
Gerrit-Reviewer: Hector Acosta <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Mar 2019 21:06:11 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] Use test -x to check for ntp-wait
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/12726 )
Change subject: Use test -x to check for ntp-wait
......................................................................
Patch Set 1: Code-Review+1
This looks good to me. Quick question: Is this a cosmetic issue or is there an actual failure case? I'm wondering if we want a JIRA for this. If it is cosmetic, then I think we can skip the JIRA.
--
To view, visit http://gerrit.cloudera.org:8080/12726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I53c63dfa651ac242050171da70540d24c4caf32c
Gerrit-Change-Number: 12726
Gerrit-PatchSet: 1
Gerrit-Owner: Hector Acosta <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Wed, 13 Mar 2019 17:48:57 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] Use test -x to check for ntp-wait
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12726 )
Change subject: Use test -x to check for ntp-wait
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2414/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/12726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I53c63dfa651ac242050171da70540d24c4caf32c
Gerrit-Change-Number: 12726
Gerrit-PatchSet: 1
Gerrit-Owner: Hector Acosta <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 12 Mar 2019 16:39:03 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8307 Use test -x to check for ntp-wait
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12726 )
Change subject: IMPALA-8307 Use test -x to check for ntp-wait
......................................................................
Patch Set 2: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/12726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I53c63dfa651ac242050171da70540d24c4caf32c
Gerrit-Change-Number: 12726
Gerrit-PatchSet: 2
Gerrit-Owner: Hector Acosta <he...@cloudera.com>
Gerrit-Reviewer: Hector Acosta <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Wed, 27 Mar 2019 00:43:43 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8307 Use test -x to check for ntp-wait
Posted by "Hector Acosta (Code Review)" <ge...@cloudera.org>.
Hector Acosta has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/12726 )
Change subject: IMPALA-8307 Use test -x to check for ntp-wait
......................................................................
IMPALA-8307 Use test -x to check for ntp-wait
Running ntp-wait --help can return 0 or 1 depending on the
output of ntpq. For example:
$ sudo ntp-wait --help 2> /dev/null; echo $?
0
$ sudo killall ntpd
$ sudo ntp-wait --help 2> /dev/null; echo $?
1
This commit instead tests whether ntp-wait exists and is executable to
determine if it's installed.
Change-Id: I53c63dfa651ac242050171da70540d24c4caf32c
---
M testdata/cluster/admin
1 file changed, 10 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/12726/2
--
To view, visit http://gerrit.cloudera.org:8080/12726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I53c63dfa651ac242050171da70540d24c4caf32c
Gerrit-Change-Number: 12726
Gerrit-PatchSet: 2
Gerrit-Owner: Hector Acosta <he...@cloudera.com>
Gerrit-Reviewer: Hector Acosta <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
[Impala-ASF-CR] IMPALA-8307 Use test -x to check for ntp-wait
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/12726 )
Change subject: IMPALA-8307 Use test -x to check for ntp-wait
......................................................................
Patch Set 2: Code-Review+2
Thanks!
--
To view, visit http://gerrit.cloudera.org:8080/12726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I53c63dfa651ac242050171da70540d24c4caf32c
Gerrit-Change-Number: 12726
Gerrit-PatchSet: 2
Gerrit-Owner: Hector Acosta <he...@cloudera.com>
Gerrit-Reviewer: Hector Acosta <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Mar 2019 20:28:41 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] Use test -x to check for ntp-wait
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/12726 )
Change subject: Use test -x to check for ntp-wait
......................................................................
Patch Set 1:
> > This looks good to me. Quick question: Is this a cosmetic issue
> or
> > is there an actual failure case? I'm wondering if we want a JIRA
> > for this. If it is cosmetic, then I think we can skip the JIRA.
>
> There's a failure case:
>
> ntp-wait runs `/usr/sbin/ntpq -c "rv 0"` regardless of whether
> --help is used or not.. Its exit status depends on the output of
> ntpq.
>
> So for example:
> [root@8280aa22322d /]# ntp-wait --help ; echo $?
> /usr/sbin/ntp-wait version [unknown] calling Getopt::Std::getopts
> (version 1.07 [paranoid]),
> running under Perl version 5.16.3.
>
> Usage: ntp-wait [-OPTIONS [-MORE_OPTIONS]] [--] [PROGRAM_ARG1 ...]
>
> The following single-character options are accepted:
> With arguments: -n -s
> Boolean (without arguments): -v
>
> Options may be merged together. -- stops processing of options.
> Space is not required between options and their arguments.
> [Now continuing due to backward compatibility and excessive
> paranoia.
> See 'perldoc Getopt::Std' about $Getopt::Std::STANDARD_HELP_VERSION.]
> 1
> [root@8280aa22322d /]# ntpd
> [root@8280aa22322d /]# ntp-wait --help ; echo $?
> /usr/sbin/ntp-wait version [unknown] calling Getopt::Std::getopts
> (version 1.07 [paranoid]),
> running under Perl version 5.16.3.
>
> Usage: ntp-wait [-OPTIONS [-MORE_OPTIONS]] [--] [PROGRAM_ARG1 ...]
>
> The following single-character options are accepted:
> With arguments: -n -s
> Boolean (without arguments): -v
>
> Options may be merged together. -- stops processing of options.
> Space is not required between options and their arguments.
> [Now continuing due to backward compatibility and excessive
> paranoia.
> See 'perldoc Getopt::Std' about $Getopt::Std::STANDARD_HELP_VERSION.]
> 0
>
>
> It also looks like --help is simply ignored.
Thanks! Given there is a failure case, can you file an IMPALA jira that ntp-wait --help does not always return 0? Then, update the commit message to reference that JIRA.
Please include in the commit message your conclusion that ntp-wait --help still runs ntpq and can have a non-zero return code.
Good catch!
--
To view, visit http://gerrit.cloudera.org:8080/12726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I53c63dfa651ac242050171da70540d24c4caf32c
Gerrit-Change-Number: 12726
Gerrit-PatchSet: 1
Gerrit-Owner: Hector Acosta <he...@cloudera.com>
Gerrit-Reviewer: Hector Acosta <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Wed, 13 Mar 2019 20:53:06 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8307 Use test -x to check for ntp-wait
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12726 )
Change subject: IMPALA-8307 Use test -x to check for ntp-wait
......................................................................
Patch Set 2:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3959/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/12726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I53c63dfa651ac242050171da70540d24c4caf32c
Gerrit-Change-Number: 12726
Gerrit-PatchSet: 2
Gerrit-Owner: Hector Acosta <he...@cloudera.com>
Gerrit-Reviewer: Hector Acosta <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Tue, 26 Mar 2019 20:15:14 +0000
Gerrit-HasComments: No