Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/15219 )
Change subject: IMPALA-9384: Improve Impala shell usability by automatically enable live_progress in the interactive mode ...................................................................... Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/15219/1/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/15219/1/shell/impala_shell.py@1742 PS1, Line 1742: config_file_live_progress = s_options.get('live_progress') > I think I didn't get what you mean here. can you please elaborate? thanks Line 1731 set config_file_live_progress = None Line 1737 puts us in a loop through config files - Say first config file has a value for 'live_progress', now on Line 1742 config_file_live_progress=True - Say second config file does not have a value for 'live_progress', now on Line 1742 config_file_live_progress=None again So config_file_live_progress gets set then overwritten, that seems weird. (Probably this doesn't matter if you rewrite as dknupp suggests) http://gerrit.cloudera.org:8080/#/c/15219/1/shell/impala_shell.py@1754 PS1, Line 1754: if not options.query and not options.query_file and config_file_live_progress is None: > I think we are doing the if statement at main() and the self.interactive is OK -- To view, visit http://gerrit.cloudera.org:8080/15219 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3765b775f663fa227e59728acffe4d5ea9a5e2d3 Gerrit-Change-Number: 15219 Gerrit-PatchSet: 1 Gerrit-Owner: Alice Fan <fan...@gmail.com> Gerrit-Reviewer: Alice Fan <fan...@gmail.com> Gerrit-Reviewer: Andrew Sherman <asher...@cloudera.com> Gerrit-Reviewer: David Knupp <dkn...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Fri, 28 Feb 2020 22:20:08 +0000 Gerrit-HasComments: Yes
