[Impala-ASF-CR] Use test -x to check for ntp-wait
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 Gerrit-Reviewer: Hector Acosta Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Wed, 13 Mar 2019 20:53:06 + Gerrit-HasComments: No
[Impala-ASF-CR] Use test -x to check for ntp-wait
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 Gerrit-Reviewer: Hector Acosta Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Wed, 13 Mar 2019 20:05:36 + Gerrit-HasComments: No
[Impala-ASF-CR] Use test -x to check for ntp-wait
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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Wed, 13 Mar 2019 17:48:57 + Gerrit-HasComments: No
[Impala-ASF-CR] Use test -x to check for ntp-wait
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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 12 Mar 2019 16:39:03 + Gerrit-HasComments: No
[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
