[impala] branch master updated: IMPALA-9384: Improve Impala shell usability by enabling live_progress in interactive mode

[ta...@apache.org]

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


The following commit(s) were added to refs/heads/master by this push:
     new e1d1428  IMPALA-9384: Improve Impala shell usability by enabling live_progress in interactive mode
e1d1428 is described below

commit e1d1428181c26faa0fb2fb9a3454e00008daa2be
Author: Alice Fan <fa...@gmail.com>
AuthorDate: Mon Mar 2 19:12:17 2020 -0800

    IMPALA-9384: Improve Impala shell usability by enabling live_progress in interactive mode
    
    In order to improve usability, this patch makes Impala shell show query
    processing status while the query is running. The patch enables shell
    option live_progress by default when a user launches impala shell in
    the interactive mode. The patch also adds a new command line flag
    "--disable_live_progress", which allows a user to disable live_progress
    at runtime. In the interactive mode, a user can disable live_progress
    by either using the command line flag or setting the option as False in
    the config file. As for in the non-interactive mode (when the -q or -f
    options are used), live reporting is not supported. Impala-shell will
    disable live_progress if the mode is detected.
    
    Testing:
    - Added and updated tests in test_shell_interactive.py and test_shell_commandline.py
    - Successfully ran all shell related tests
    
    Change-Id: I3765b775f663fa227e59728acffe4d5ea9a5e2d3
    Reviewed-on: http://gerrit.cloudera.org:8080/15219
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
    Reviewed-by: Tim Armstrong <ta...@cloudera.com>
---
 shell/impala_shell.py                 |  9 +++++++--
 shell/impala_shell_config_defaults.py |  2 +-
 shell/option_parser.py                | 20 +++++++++++++++++++-
 tests/shell/test_shell_commandline.py |  4 ++--
 tests/shell/test_shell_interactive.py | 19 ++++++++++++++++++-
 5 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/shell/impala_shell.py b/shell/impala_shell.py
index 74a4c3b..4c3a3cf 100755
--- a/shell/impala_shell.py
+++ b/shell/impala_shell.py
@@ -1839,9 +1839,14 @@ def impala_shell_main():
   query_options.update(
      [(k.upper(), v) for k, v in parse_variables(options.query_options).items()])
 
+  # Non-interactive mode
   if options.query or options.query_file:
-    if options.live_progress or options.live_summary:
-      print_to_stderr("Error: Live reporting is available for interactive mode only.")
+    # Impala shell will disable live_progress if non-interactive mode is detected.
+    options.live_progress = False
+    print_to_stderr("Warning: live_progress only applies to interactive shell sessions, "
+                    "and is being skipped for now.")
+    if options.live_summary:
+      print_to_stderr("Error: live_summary is available for interactive mode only.")
       raise FatalShellException()
 
     if execute_queries_non_interactive_mode(options, query_options):
diff --git a/shell/impala_shell_config_defaults.py b/shell/impala_shell_config_defaults.py
index 98342db..73bfbcb 100644
--- a/shell/impala_shell_config_defaults.py
+++ b/shell/impala_shell_config_defaults.py
@@ -39,7 +39,7 @@ impala_shell_defaults = {
             'output_delimiter': '\\t',
             'output_file': None,
             'print_header': False,
-            'live_progress': False,
+            'live_progress': True,  # The option only applies to interactive shell session
             'live_summary': False,
             'query': None,
             'query_file': None,
diff --git a/shell/option_parser.py b/shell/option_parser.py
index 583813c..3573993 100755
--- a/shell/option_parser.py
+++ b/shell/option_parser.py
@@ -233,7 +233,13 @@ def get_option_parser(defaults):
   parser.add_option("--live_summary", dest="live_summary", action="store_true",
                     help="Print a query summary every 1s while the query is running.")
   parser.add_option("--live_progress", dest="live_progress", action="store_true",
-                    help="Print a query progress every 1s while the query is running.")
+                    help="Print a query progress every 1s while the query is running."
+                         " The default value of the flag is True in the interactive mode."
+                         " If live_progress is set to False in a config file, this flag"
+                         " will override it")
+  parser.add_option("--disable_live_progress", dest="live_progress", action="store_false",
+                    help="A command line flag allows users to disable live_progress in"
+                         " the interactive mode.")
   parser.add_option("--auth_creds_ok_in_clear", dest="creds_ok_in_clear",
                     action="store_true", help="If set, LDAP authentication " +
                     "may be used with an insecure connection to Impala. " +
@@ -298,10 +304,22 @@ def get_option_parser(defaults):
     # (print quiet is false since verbose is true)
     if option == parser.get_option('--quiet'):
       option.help += " [default: %s]" % (not defaults['verbose'])
+    # print default value of disable_live_progress in the help messages as opposite
+    # value for default value of live_progress
+    # (print disable_live_progress is false since live_progress is true)
+    elif option == parser.get_option('--disable_live_progress'):
+      option.help += " [default: %s]" % (not defaults['live_progress'])
     elif option != parser.get_option('--help') and option.help is not SUPPRESS_HELP:
       # don't want to print default value for help or options without help text
       option.help += " [default: %default]"
 
+  # mutually exclusive flags should not be used in the same time
+  if '--live_progress' in sys.argv and '--disable_live_progress' in sys.argv:
+    parser.error("options --live_progress and --disable_live_progress are mutually "
+                 "exclusive")
+  if '--verbose' in sys.argv and '--quiet' in sys.argv:
+    parser.error("options --verbose and --quiet are mutually exclusive")
+
   parser.set_defaults(**defaults)
 
   return parser
diff --git a/tests/shell/test_shell_commandline.py b/tests/shell/test_shell_commandline.py
index d4417c0..85294f1 100644
--- a/tests/shell/test_shell_commandline.py
+++ b/tests/shell/test_shell_commandline.py
@@ -582,10 +582,10 @@ class TestImpalaShell(ImpalaTestSuite):
     assert 'WARNING:' not in result.stderr, \
       "A valid config file should not trigger any warning: {0}".format(result.stderr)
     # Negative Tests
-    # specified config file with live_progress enabled for non interactive mode
+    # specified config file with live_summary enabled for non interactive mode
     args = ['--config_file=%s/good_impalarc3' % QUERY_FILE_PATH, '--query=select 3']
     result = run_impala_shell_cmd(vector, args, expect_success=False)
-    assert 'Live reporting is available for interactive mode only' in result.stderr
+    assert 'live_summary is available for interactive mode only' in result.stderr
     # testing config file related warning messages
     args = ['--config_file=%s/impalarc_with_warnings2' % QUERY_FILE_PATH]
     result = run_impala_shell_cmd(
diff --git a/tests/shell/test_shell_interactive.py b/tests/shell/test_shell_interactive.py
index 109ca47..cac263b 100755
--- a/tests/shell/test_shell_interactive.py
+++ b/tests/shell/test_shell_interactive.py
@@ -126,7 +126,7 @@ class TestImpalaShellInteractive(ImpalaTestSuite):
     proc = pexpect.spawn(shell_cmd[0], shell_cmd[1:])
     proc.expect(":{0}] default>".format(get_impalad_port(vector)))
     self._expect_with_cmd(proc, "set", vector,
-        ("LIVE_PROGRESS: False", "LIVE_SUMMARY: False"))
+        ("LIVE_PROGRESS: True", "LIVE_SUMMARY: False"))
     self._expect_with_cmd(proc, "set live_progress=true", vector)
     self._expect_with_cmd(proc, "set", vector,
         ("LIVE_PROGRESS: True", "LIVE_SUMMARY: False"))
@@ -531,6 +531,23 @@ class TestImpalaShellInteractive(ImpalaTestSuite):
     # Verify that query options under [impala] override those under [impala.query_options]
     assert "\tDEFAULT_FILE_FORMAT: avro" in result.stdout
 
+  def test_commandline_flag_disable_live_progress(self, vector):
+    """Test the command line flag disable_live_progress with live_progress."""
+    # By default, shell option live_progress is set to True in the interactive mode.
+    cmds = "set all;"
+    result = run_impala_shell_interactive(vector, cmds)
+    assert "\tLIVE_PROGRESS: True" in result.stdout
+    # override the default option through command line argument.
+    args = ['--disable_live_progress']
+    result = run_impala_shell_interactive(vector, cmds, shell_args=args)
+    assert "\tLIVE_PROGRESS: False" in result.stdout
+    # set live_progress as True with config file.
+    # override the option in config file through command line argument.
+    rcfile_path = os.path.join(QUERY_FILE_PATH, 'good_impalarc3')
+    args = ['--disable_live_progress', '--config_file=%s' % rcfile_path]
+    result = run_impala_shell_interactive(vector, cmds, shell_args=args)
+    assert "\tLIVE_PROGRESS: False" in result.stdout
+
   def test_live_option_configuration(self, vector):
     """Test the optional configuration file with live_progress and live_summary."""
     # Positive tests