You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Anonymous Coward (Code Review)" <ge...@cloudera.org> on 2018/07/10 14:53:19 UTC

[Impala-ASF-CR] IMPALA-1624: Allow toggling some command-line options inside impala-shell

le.nghia@cloudera.com has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10900


Change subject: IMPALA-1624: Allow toggling some command-line options inside impala-shell
......................................................................

IMPALA-1624: Allow toggling some command-line options inside impala-shell

This change provides a way to modify command-line options like -B,
--output_file and --delimiter inside impala-shell without quitting
the shell and then restarting again.

Testing:
Added tests for all new options in test_shell_interactive.py

Change-Id: Id8d4487c24f24806223bfd5c54336914e3afd763
---
M shell/impala_shell.py
M tests/shell/test_shell_interactive.py
2 files changed, 54 insertions(+), 1 deletion(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/10900/1
-- 
To view, visit http://gerrit.cloudera.org:8080/10900
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id8d4487c24f24806223bfd5c54336914e3afd763
Gerrit-Change-Number: 10900
Gerrit-PatchSet: 1
Gerrit-Owner: le.nghia@cloudera.com

[Impala-ASF-CR] IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell

Posted by "Le Minh Nghia (Code Review)" <ge...@cloudera.org>.
Hello Zoltan Borok-Nagy, Csaba Ringhofer, Jinchul Kim, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/10900

to look at the new patch set (#5).

Change subject: IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell
......................................................................

IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell

This change provides a way to modify command-line options like -B,
--output_file and --delimiter inside impala-shell without quitting
the shell and then restarting again.
Also fixed IMPALA-7286: command "unset" does not work for shell options

Testing:
Added tests for all new options in test_shell_interactive.py

Change-Id: Id8d4487c24f24806223bfd5c54336914e3afd763
---
M shell/impala_shell.py
M tests/shell/test_shell_interactive.py
2 files changed, 72 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/10900/5
-- 
To view, visit http://gerrit.cloudera.org:8080/10900
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id8d4487c24f24806223bfd5c54336914e3afd763
Gerrit-Change-Number: 10900
Gerrit-PatchSet: 5
Gerrit-Owner: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Jinchul Kim <ji...@gmail.com>
Gerrit-Reviewer: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10900 )

Change subject: IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell
......................................................................


Patch Set 7:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/10900/7/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/10900/7/shell/impala_shell.py@137
PS7, Line 137: E
flake8: E201 whitespace after '['


http://gerrit.cloudera.org:8080/#/c/10900/7/shell/impala_shell.py@137
PS7, Line 137: x
flake8: E202 whitespace before ']'



-- 
To view, visit http://gerrit.cloudera.org:8080/10900
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id8d4487c24f24806223bfd5c54336914e3afd763
Gerrit-Change-Number: 10900
Gerrit-PatchSet: 7
Gerrit-Owner: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jinchul Kim <ji...@gmail.com>
Gerrit-Reviewer: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 31 Jul 2018 13:36:04 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10900 )

Change subject: IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell
......................................................................


Patch Set 8:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2889/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/10900
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id8d4487c24f24806223bfd5c54336914e3afd763
Gerrit-Change-Number: 10900
Gerrit-PatchSet: 8
Gerrit-Owner: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jinchul Kim <ji...@gmail.com>
Gerrit-Reviewer: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 31 Jul 2018 17:22:50 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell

Posted by "Le Minh Nghia (Code Review)" <ge...@cloudera.org>.
Hello Zoltan Borok-Nagy, Csaba Ringhofer, Jinchul Kim, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/10900

to look at the new patch set (#4).

Change subject: IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell
......................................................................

IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell

This change provides a way to modify command-line options like -B,
--output_file and --delimiter inside impala-shell without quitting
the shell and then restarting again.
Also fixed IMPALA-7286: comment "unset" does not work for shell options

Testing:
Added tests for all new options in test_shell_interactive.py

Change-Id: Id8d4487c24f24806223bfd5c54336914e3afd763
---
M shell/impala_shell.py
M tests/shell/test_shell_interactive.py
2 files changed, 72 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/10900/4
-- 
To view, visit http://gerrit.cloudera.org:8080/10900
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id8d4487c24f24806223bfd5c54336914e3afd763
Gerrit-Change-Number: 10900
Gerrit-PatchSet: 4
Gerrit-Owner: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Jinchul Kim <ji...@gmail.com>
Gerrit-Reviewer: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10900 )

Change subject: IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell
......................................................................


Patch Set 6: Code-Review+1


-- 
To view, visit http://gerrit.cloudera.org:8080/10900
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id8d4487c24f24806223bfd5c54336914e3afd763
Gerrit-Change-Number: 10900
Gerrit-PatchSet: 6
Gerrit-Owner: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Jinchul Kim <ji...@gmail.com>
Gerrit-Reviewer: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 17 Jul 2018 13:23:18 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-1624: Allow toggling some command-line options inside impala-shell

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/10900 )

Change subject: IMPALA-1624: Allow toggling some command-line options inside impala-shell
......................................................................


Patch Set 2:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/10900/2/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/10900/2/shell/impala_shell.py@131
PS2, Line 131:   # Valid true value for some shell options
nit: we typically finish comments with '.' in Impala
I would also rephrase the sentence to something like "Strings that are interpreted as True for some boolean options."


http://gerrit.cloudera.org:8080/#/c/10900/2/shell/impala_shell.py@177
PS2, Line 177: \
nit: please add a space before the last \ - I think that we do this everywhere in Impala.


http://gerrit.cloudera.org:8080/#/c/10900/2/shell/impala_shell.py@743
PS2, Line 743:     elif not self._handle_unset_shell_options(option):
Please print a message when the unset succeeds.


http://gerrit.cloudera.org:8080/#/c/10900/1/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:

http://gerrit.cloudera.org:8080/#/c/10900/1/tests/shell/test_shell_interactive.py@125
PS1, Line 125:     p2 = ImpalaShell()
> Done
Thanks for doing IMPALA-7286! Please also mention this change in the commit message.


http://gerrit.cloudera.org:8080/#/c/10900/2/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:

http://gerrit.cloudera.org:8080/#/c/10900/2/tests/shell/test_shell_interactive.py@94
PS2, Line 94:     p.send_cmd("set delimiter=,")
Can we keep the default delimiter in this test to cover more scenarios by the tests?



-- 
To view, visit http://gerrit.cloudera.org:8080/10900
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id8d4487c24f24806223bfd5c54336914e3afd763
Gerrit-Change-Number: 10900
Gerrit-PatchSet: 2
Gerrit-Owner: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Jinchul Kim <ji...@gmail.com>
Gerrit-Reviewer: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 13 Jul 2018 15:46:02 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-1624: Allow toggling some command-line options inside impala-shell

Posted by "Le Minh Nghia (Code Review)" <ge...@cloudera.org>.
Hello Zoltan Borok-Nagy, Csaba Ringhofer, Jinchul Kim, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/10900

to look at the new patch set (#2).

Change subject: IMPALA-1624: Allow toggling some command-line options inside impala-shell
......................................................................

IMPALA-1624: Allow toggling some command-line options inside impala-shell

This change provides a way to modify command-line options like -B,
--output_file and --delimiter inside impala-shell without quitting
the shell and then restarting again.

Testing:
Added tests for all new options in test_shell_interactive.py

Change-Id: Id8d4487c24f24806223bfd5c54336914e3afd763
---
M shell/impala_shell.py
M tests/shell/test_shell_interactive.py
2 files changed, 72 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/10900/2
-- 
To view, visit http://gerrit.cloudera.org:8080/10900
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id8d4487c24f24806223bfd5c54336914e3afd763
Gerrit-Change-Number: 10900
Gerrit-PatchSet: 2
Gerrit-Owner: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Jinchul Kim <ji...@gmail.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10900 )

Change subject: IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell
......................................................................


Patch Set 8: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/10900
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id8d4487c24f24806223bfd5c54336914e3afd763
Gerrit-Change-Number: 10900
Gerrit-PatchSet: 8
Gerrit-Owner: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jinchul Kim <ji...@gmail.com>
Gerrit-Reviewer: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 31 Jul 2018 20:35:26 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell

Posted by "Le Minh Nghia (Code Review)" <ge...@cloudera.org>.
Hello Fredy Wijaya, Zoltan Borok-Nagy, Attila Jeges, Csaba Ringhofer, Jinchul Kim, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/10900

to look at the new patch set (#7).

Change subject: IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell
......................................................................

IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell

This change provides a way to modify command-line options like -B,
--output_file and --delimiter inside impala-shell without quitting
the shell and then restarting again.
Also fixed IMPALA-7286: command "unset" does not work for shell options

Testing:
Added tests for all new options in test_shell_interactive.py
Tested on Python 2.6 and Python 2.7

Change-Id: Id8d4487c24f24806223bfd5c54336914e3afd763
---
M shell/impala_shell.py
M tests/shell/test_shell_interactive.py
2 files changed, 75 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/10900/7
-- 
To view, visit http://gerrit.cloudera.org:8080/10900
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id8d4487c24f24806223bfd5c54336914e3afd763
Gerrit-Change-Number: 10900
Gerrit-PatchSet: 7
Gerrit-Owner: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Jinchul Kim <ji...@gmail.com>
Gerrit-Reviewer: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell

Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/10900 )

Change subject: IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell
......................................................................


Patch Set 7: Code-Review+2

> Patch Set 7:
> 
> Some flake8 warnings:
> flake8: E201 whitespace after '['
> flake8: E202 whitespace before ']'
> 
> The flake8 warnings don't seem to correspond with the diff. Tim, do you want to take a look at this?

From the mailing list, it looks like a bug in automatic code review pre-commit job for flake8. This patch looks good. So, I'm giving it a +2.


-- 
To view, visit http://gerrit.cloudera.org:8080/10900
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id8d4487c24f24806223bfd5c54336914e3afd763
Gerrit-Change-Number: 10900
Gerrit-PatchSet: 7
Gerrit-Owner: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jinchul Kim <ji...@gmail.com>
Gerrit-Reviewer: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 31 Jul 2018 17:06:16 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10900 )

Change subject: IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell
......................................................................


Patch Set 8: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/10900
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id8d4487c24f24806223bfd5c54336914e3afd763
Gerrit-Change-Number: 10900
Gerrit-PatchSet: 8
Gerrit-Owner: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jinchul Kim <ji...@gmail.com>
Gerrit-Reviewer: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 31 Jul 2018 17:22:48 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10900 )

Change subject: IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell
......................................................................

IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell

This change provides a way to modify command-line options like -B,
--output_file and --delimiter inside impala-shell without quitting
the shell and then restarting again.
Also fixed IMPALA-7286: command "unset" does not work for shell options

Testing:
Added tests for all new options in test_shell_interactive.py
Tested on Python 2.6 and Python 2.7

Change-Id: Id8d4487c24f24806223bfd5c54336914e3afd763
Reviewed-on: http://gerrit.cloudera.org:8080/10900
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M shell/impala_shell.py
M tests/shell/test_shell_interactive.py
2 files changed, 75 insertions(+), 4 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/10900
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Id8d4487c24f24806223bfd5c54336914e3afd763
Gerrit-Change-Number: 10900
Gerrit-PatchSet: 9
Gerrit-Owner: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jinchul Kim <ji...@gmail.com>
Gerrit-Reviewer: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell

Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/10900 )

Change subject: IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell
......................................................................


Patch Set 6:

(2 comments)

LGTM, except one tiny nit. I can give a +2 if the change was confirmed to to be tested on both Python 2.6 and 2.7.

http://gerrit.cloudera.org:8080/#/c/10900/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10900/6//COMMIT_MSG@15
PS6, Line 15: Added tests for all new options in test_shell_interactive.py
Did you test this change on Python 2.6 as well? If yes, can you update the commit message saying tested on Python 2.6 and Python 2.7?


http://gerrit.cloudera.org:8080/#/c/10900/6/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:

http://gerrit.cloudera.org:8080/#/c/10900/6/tests/shell/test_shell_interactive.py@122
PS6, Line 122: 'r'
nit: use "r" to be consistent with the code in this function.



-- 
To view, visit http://gerrit.cloudera.org:8080/10900
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id8d4487c24f24806223bfd5c54336914e3afd763
Gerrit-Change-Number: 10900
Gerrit-PatchSet: 6
Gerrit-Owner: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Jinchul Kim <ji...@gmail.com>
Gerrit-Reviewer: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 27 Jul 2018 17:04:31 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell

Posted by "Le Minh Nghia (Code Review)" <ge...@cloudera.org>.
Hello Zoltan Borok-Nagy, Csaba Ringhofer, Jinchul Kim, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/10900

to look at the new patch set (#3).

Change subject: IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell
......................................................................

IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell

This change provides a way to modify command-line options like -B,
--output_file and --delimiter inside impala-shell without quitting
the shell and then restarting again.
Also fixed IMPALA-7286

Testing:
Added tests for all new options in test_shell_interactive.py

Change-Id: Id8d4487c24f24806223bfd5c54336914e3afd763
---
M shell/impala_shell.py
M tests/shell/test_shell_interactive.py
2 files changed, 72 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/10900/3
-- 
To view, visit http://gerrit.cloudera.org:8080/10900
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id8d4487c24f24806223bfd5c54336914e3afd763
Gerrit-Change-Number: 10900
Gerrit-PatchSet: 3
Gerrit-Owner: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Jinchul Kim <ji...@gmail.com>
Gerrit-Reviewer: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell

Posted by "Le Minh Nghia (Code Review)" <ge...@cloudera.org>.
Le Minh Nghia has posted comments on this change. ( http://gerrit.cloudera.org:8080/10900 )

Change subject: IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell
......................................................................


Patch Set 7:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/10900/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10900/6//COMMIT_MSG@15
PS6, Line 15: Added tests for all new options in test_shell_interactive.py
> Did you test this change on Python 2.6 as well? If yes, can you update the 
Done


http://gerrit.cloudera.org:8080/#/c/10900/5/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:

http://gerrit.cloudera.org:8080/#/c/10900/5/tests/shell/test_shell_interactive.py@91
PS5, Line 91:     """Test output rows in delimited mode"""
> nit: please add some doc comments to the tests.
Done


http://gerrit.cloudera.org:8080/#/c/10900/6/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:

http://gerrit.cloudera.org:8080/#/c/10900/6/tests/shell/test_shell_interactive.py@122
PS6, Line 122: "r"
> nit: use "r" to be consistent with the code in this function.
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/10900
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id8d4487c24f24806223bfd5c54336914e3afd763
Gerrit-Change-Number: 10900
Gerrit-PatchSet: 7
Gerrit-Owner: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jinchul Kim <ji...@gmail.com>
Gerrit-Reviewer: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 31 Jul 2018 13:58:09 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/10900 )

Change subject: IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell
......................................................................


Patch Set 4: Code-Review+1

(1 comment)

Please fix my comment about the commit message, otherwise looks good to me.

I will be on vacation next week, so please find someone who will look through the change and gives the +2.

http://gerrit.cloudera.org:8080/#/c/10900/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10900/4//COMMIT_MSG@12
PS4, Line 12: comment
typo: Command



-- 
To view, visit http://gerrit.cloudera.org:8080/10900
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id8d4487c24f24806223bfd5c54336914e3afd763
Gerrit-Change-Number: 10900
Gerrit-PatchSet: 4
Gerrit-Owner: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Jinchul Kim <ji...@gmail.com>
Gerrit-Reviewer: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Sun, 15 Jul 2018 15:31:24 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell

Posted by "Le Minh Nghia (Code Review)" <ge...@cloudera.org>.
Hello Zoltan Borok-Nagy, Csaba Ringhofer, Jinchul Kim, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/10900

to look at the new patch set (#6).

Change subject: IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell
......................................................................

IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell

This change provides a way to modify command-line options like -B,
--output_file and --delimiter inside impala-shell without quitting
the shell and then restarting again.
Also fixed IMPALA-7286: command "unset" does not work for shell options

Testing:
Added tests for all new options in test_shell_interactive.py

Change-Id: Id8d4487c24f24806223bfd5c54336914e3afd763
---
M shell/impala_shell.py
M tests/shell/test_shell_interactive.py
2 files changed, 75 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/10900/6
-- 
To view, visit http://gerrit.cloudera.org:8080/10900
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id8d4487c24f24806223bfd5c54336914e3afd763
Gerrit-Change-Number: 10900
Gerrit-PatchSet: 6
Gerrit-Owner: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Jinchul Kim <ji...@gmail.com>
Gerrit-Reviewer: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10900 )

Change subject: IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell
......................................................................


Patch Set 7:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/114/ : 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/10900
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id8d4487c24f24806223bfd5c54336914e3afd763
Gerrit-Change-Number: 10900
Gerrit-PatchSet: 7
Gerrit-Owner: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jinchul Kim <ji...@gmail.com>
Gerrit-Reviewer: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 31 Jul 2018 14:11:25 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-1624: Allow toggling some command-line options inside impala-shell

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/10900 )

Change subject: IMPALA-1624: Allow toggling some command-line options inside impala-shell
......................................................................


Patch Set 1:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/10900/1/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/10900/1/shell/impala_shell.py@135
PS1, Line 135: "true", "TRUE", "True", "1"
This could be moved to a separate variable like TRUE_STRINGS or the whole logic could be moved to a function.


http://gerrit.cloudera.org:8080/#/c/10900/1/shell/impala_shell.py@137
PS1, Line 137: \\s
Will this handle other special characters like tabs and new lines?


http://gerrit.cloudera.org:8080/#/c/10900/1/shell/impala_shell.py@138
PS1, Line 138: None
I would prefer an empty string instead on 'None' to be consistent with the command line option (running the shell with argument "-o None" will create a file named None.


http://gerrit.cloudera.org:8080/#/c/10900/1/shell/impala_shell.py@1435
PS1, Line 1435: ,
I have noticed that this tip is wrong. Please correct the default to \t. It could be also useful to add a tip for the new shell options.


http://gerrit.cloudera.org:8080/#/c/10900/1/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:

http://gerrit.cloudera.org:8080/#/c/10900/1/tests/shell/test_shell_interactive.py@98
PS1, Line 98:     assert "+----------------+" not in result.stdout
Please also add a positive test (a row that should be included in the result).


http://gerrit.cloudera.org:8080/#/c/10900/1/tests/shell/test_shell_interactive.py@119
PS1, Line 119:     assert "ALGERIA" not in result.stdout
It should be also verified that the results were written to the file.


http://gerrit.cloudera.org:8080/#/c/10900/1/tests/shell/test_shell_interactive.py@125
PS1, Line 125:     p2.send_cmd("set output_file=None")
The primary way to set back options to their default should be calling "unset". This does not work for shell options at the moment, but is should be trivial to fix it (see IMPALA-7286).



-- 
To view, visit http://gerrit.cloudera.org:8080/10900
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id8d4487c24f24806223bfd5c54336914e3afd763
Gerrit-Change-Number: 10900
Gerrit-PatchSet: 1
Gerrit-Owner: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Jinchul Kim <ji...@gmail.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Jul 2018 12:42:23 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/10900 )

Change subject: IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/10900/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10900/3//COMMIT_MSG@12
PS3, Line 12: Also fixed IMPALA-7286
Please be a bit more talkative about this :) at least add the title of the Jira like "IMPALA-7286
Command "unset" does not work for shell options"


http://gerrit.cloudera.org:8080/#/c/10900/3/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/10900/3/shell/impala_shell.py@657
PS3, Line 657:       print 'Unsetting shell option %s' % token
Please move this to the calling function to be consistent with the other printed messages.



-- 
To view, visit http://gerrit.cloudera.org:8080/10900
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id8d4487c24f24806223bfd5c54336914e3afd763
Gerrit-Change-Number: 10900
Gerrit-PatchSet: 3
Gerrit-Owner: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Jinchul Kim <ji...@gmail.com>
Gerrit-Reviewer: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 13 Jul 2018 16:08:05 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell

Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/10900 )

Change subject: IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell
......................................................................


Patch Set 7:

Some flake8 warnings:
flake8: E201 whitespace after '['
flake8: E202 whitespace before ']'

The flake8 warnings don't seem to correspond with the diff. Tim, do you want to take a look at this?


-- 
To view, visit http://gerrit.cloudera.org:8080/10900
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id8d4487c24f24806223bfd5c54336914e3afd763
Gerrit-Change-Number: 10900
Gerrit-PatchSet: 7
Gerrit-Owner: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jinchul Kim <ji...@gmail.com>
Gerrit-Reviewer: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 31 Jul 2018 15:47:03 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-1624: Allow toggling some command-line options inside impala-shell

Posted by "Le Minh Nghia (Code Review)" <ge...@cloudera.org>.
Le Minh Nghia has posted comments on this change. ( http://gerrit.cloudera.org:8080/10900 )

Change subject: IMPALA-1624: Allow toggling some command-line options inside impala-shell
......................................................................


Patch Set 2:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/10900/1/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/10900/1/shell/impala_shell.py@135
PS1, Line 135: alaShell.TRUE_STRINGS, "pri
> This could be moved to a separate variable like TRUE_STRINGS or the whole l
Done


http://gerrit.cloudera.org:8080/#/c/10900/1/shell/impala_shell.py@137
PS1, Line 137: pal
> Will this handle other special characters like tabs and new lines?
Yes, it will.


http://gerrit.cloudera.org:8080/#/c/10900/1/shell/impala_shell.py@138
PS1, Line 138: RUE_
> I would prefer an empty string instead on 'None' to be consistent with the 
Done


http://gerrit.cloudera.org:8080/#/c/10900/1/shell/impala_shell.py@1435
PS1, Line 1435: 
> I have noticed that this tip is wrong. Please correct the default to \t. It
Done


http://gerrit.cloudera.org:8080/#/c/10900/1/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:

http://gerrit.cloudera.org:8080/#/c/10900/1/tests/shell/test_shell_interactive.py@98
PS1, Line 98:     assert "21,VIETNAM,2" in result.stdout
> Please also add a positive test (a row that should be included in the resul
Done


http://gerrit.cloudera.org:8080/#/c/10900/1/tests/shell/test_shell_interactive.py@119
PS1, Line 119:     assert "VIETNAM" not in result.stdout
> It should be also verified that the results were written to the file.
Done


http://gerrit.cloudera.org:8080/#/c/10900/1/tests/shell/test_shell_interactive.py@125
PS1, Line 125:     p2 = ImpalaShell()
> The primary way to set back options to their default should be calling "uns
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/10900
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id8d4487c24f24806223bfd5c54336914e3afd763
Gerrit-Change-Number: 10900
Gerrit-PatchSet: 2
Gerrit-Owner: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Jinchul Kim <ji...@gmail.com>
Gerrit-Reviewer: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 13 Jul 2018 15:20:18 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10900 )

Change subject: IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell
......................................................................


Patch Set 5:

(1 comment)

Seems good to me, only missed some doc comments.

http://gerrit.cloudera.org:8080/#/c/10900/5/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:

http://gerrit.cloudera.org:8080/#/c/10900/5/tests/shell/test_shell_interactive.py@91
PS5, Line 91:     p  = ImpalaShell()
nit: please add some doc comments to the tests.



-- 
To view, visit http://gerrit.cloudera.org:8080/10900
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id8d4487c24f24806223bfd5c54336914e3afd763
Gerrit-Change-Number: 10900
Gerrit-PatchSet: 5
Gerrit-Owner: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Jinchul Kim <ji...@gmail.com>
Gerrit-Reviewer: Le Minh Nghia <le...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 16 Jul 2018 15:07:20 +0000
Gerrit-HasComments: Yes