This is an automated email from the ASF dual-hosted git repository.
tarmstrong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 561158b30656b97ccb6ec83052b62c476e0873ed
Author: Fredy Wijaya <fwij...@cloudera.com>
AuthorDate: Fri Jan 18 12:57:37 2019 -0800
IMPALA-3323: Fix unrecognizable shell option when --config_file is specified
Impala shell defines a dictionary of default values for some shell
options. Before this patch, the logic for --config_file checks if
a shell option exists by using the default value dictionary, which
does not contain the exhaustive list of shell options. This causes
a valid option in the Impala shell config file to be treated as
unrecognizable shell option due to the option not having a default
value. The patch fixes the issue by changing the logic that checks
for the existence of an option using the option list from optparse.
The patch also fixes the missing dest parameter for ldap_password_cmd
option.
Testing:
- Updated test_shell_commandline::test_config_file
- Ran all shell tests
Change-Id: Iff371d038fa77ba659e9b7c7a4ed5b374237f2ea
Reviewed-on: http://gerrit.cloudera.org:8080/12245
Reviewed-by: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Tested-by: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
---
shell/impala_shell.py | 3 ++-
shell/option_parser.py | 30 +++++++++++++++++++-----------
tests/shell/good_impalarc | 1 +
tests/shell/test_shell_commandline.py | 1 +
4 files changed, 23 insertions(+), 12 deletions(-)
diff --git a/shell/impala_shell.py b/shell/impala_shell.py
index 7471c2a..5fb2e6c 100755
--- a/shell/impala_shell.py
+++ b/shell/impala_shell.py
@@ -1629,7 +1629,8 @@ if __name__ == "__main__":
# default shell options loaded in from impala_shell_config_defaults.py
# options defaults overwritten by those in config file
try:
- loaded_shell_options, query_options = get_config_from_file(config_to_load)
+ loaded_shell_options, query_options = get_config_from_file(config_to_load,
+
parser.option_list)
impala_shell_defaults.update(loaded_shell_options)
except Exception, e:
print_to_stderr(e)
diff --git a/shell/option_parser.py b/shell/option_parser.py
index ccae53b..04d69d4 100755
--- a/shell/option_parser.py
+++ b/shell/option_parser.py
@@ -32,14 +32,17 @@ import sys
from impala_shell_config_defaults import impala_shell_defaults
from optparse import OptionParser
+
class ConfigFileFormatError(Exception):
"""Raised when the config file cannot be read by ConfigParser."""
pass
+
class InvalidOptionValueError(Exception):
"""Raised when an option contains an invalid value."""
pass
+
def parse_bool_option(value):
"""Returns True for '1' and 'True', and False for '0' and 'False'.
Throws ValueError for other values.
@@ -49,21 +52,24 @@ def parse_bool_option(value):
elif value.lower() in ["false", "0"]:
return False
else:
- raise InvalidOptionValueError("Unexpected value in configuration file. '"
+ value \
+ raise InvalidOptionValueError("Unexpected value in configuration file. '"
+ value
+ "' is not a valid value for a boolean option.")
-def parse_shell_options(options, defaults):
+
+def parse_shell_options(options, defaults, option_list):
"""Filters unknown options and converts some values from string to their
corresponding
- python types (booleans and None).
+ python types (booleans and None). 'option_list' contains the list of
valid options,
+ and 'defaults' is used to deduce the type of some options (only bool at
the moment).
Returns a dictionary with option names as keys and option values as
values.
"""
result = {}
+ option_dests = set(opt.dest for opt in option_list)
for option, value in options:
- if option not in defaults:
- print >> sys.stderr, "WARNING: Unable to read configuration file
correctly. " \
- "Ignoring unrecognized config option: '%s'\n" % option
- elif isinstance(defaults[option], bool):
+ if option not in option_dests:
+ print >> sys.stderr, "WARNING: Unable to read configuration file
correctly. " \
+ "Ignoring unrecognized config option: '%s'\n" % option
+ elif isinstance(defaults.get(option), bool):
result[option] = parse_bool_option(value)
elif value.lower() == "none":
result[option] = None
@@ -71,7 +77,8 @@ def parse_shell_options(options, defaults):
result[option] = value
return result
-def get_config_from_file(config_filename):
+
+def get_config_from_file(config_filename, option_list):
"""Reads contents of configuration file
Two config sections are supported:
@@ -90,7 +97,6 @@ def get_config_from_file(config_filename):
Returns a pair of dictionaries (shell_options, query_options), with option
names
as keys and option values as values.
"""
-
config = ConfigParser.ConfigParser()
try:
config.read(config_filename)
@@ -100,7 +106,8 @@ def get_config_from_file(config_filename):
shell_options = {}
if config.has_section("impala"):
- shell_options = parse_shell_options(config.items("impala"),
impala_shell_defaults)
+ shell_options = parse_shell_options(config.items("impala"),
impala_shell_defaults,
+ option_list)
if "config_file" in shell_options:
print >> sys.stderr, "WARNING: Option 'config_file' can be only set from
shell."
shell_options["config_file"] = config_filename
@@ -114,6 +121,7 @@ def get_config_from_file(config_filename):
return shell_options, query_options
+
def get_option_parser(defaults):
"""Creates OptionParser and adds shell options (flags)
@@ -207,7 +215,7 @@ def get_option_parser(defaults):
"may be used with an insecure connection to Impala. " +
"WARNING: Authentication credentials will therefore be
sent " +
"unencrypted, and may be vulnerable to attack.")
- parser.add_option("--ldap_password_cmd",
+ parser.add_option("--ldap_password_cmd", dest="ldap_password_cmd",
help="Shell command to run to retrieve the LDAP password")
parser.add_option("--var", dest="keyval", action="append",
help="Defines a variable to be used within the Impala
session."
diff --git a/tests/shell/good_impalarc b/tests/shell/good_impalarc
index ea6a276..3310a80 100644
--- a/tests/shell/good_impalarc
+++ b/tests/shell/good_impalarc
@@ -1,2 +1,3 @@
[impala]
query=select 1
+creds_ok_in_clear=true
\ No newline at end of file
diff --git a/tests/shell/test_shell_commandline.py
b/tests/shell/test_shell_commandline.py
index 66c28e9..86b7f2a 100644
--- a/tests/shell/test_shell_commandline.py
+++ b/tests/shell/test_shell_commandline.py
@@ -471,6 +471,7 @@ class TestImpalaShell(ImpalaTestSuite):
# Positive tests
args = '--config_file=%s/good_impalarc' % QUERY_FILE_PATH
result = run_impala_shell_cmd(args)
+ assert 'WARNING:' not in result.stderr
assert 'Query: select 1' in result.stderr
# override option in config file through command line
