You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Fredy Wijaya (Code Review)" <ge...@cloudera.org> on 2019/03/21 17:18:15 UTC

[Impala-ASF-CR] IMPALA-8330: Impala shell config file should use flag names

Fredy Wijaya has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12823


Change subject: IMPALA-8330: Impala shell config file should use flag names
......................................................................

IMPALA-8330: Impala shell config file should use flag names

This patch changes the file format in Impala shell config file to accept
both short and long flag names instead of dest names because dest names
are internal to Impala shell. This patch does not change the format and
behavior of Impala shell config file with [impala.query_options] section.

Format:
[impala]
flag_name=flag_value

Example:
[impala]
; This is long flag.
query=select 1
; This is short flag.
Q=DEFAULT_FILE_FORMAT=parquet
; Flags can be repeated.
var=msg1=hello
var=msg2=world

Testing:
- Ran all E2E shell tests

Change-Id: Ic43603c1b538af08fddcab1b2c1f6ad1af1a6cb9
---
M shell/impala_shell.py
M shell/option_parser.py
M tests/shell/good_impalarc
3 files changed, 27 insertions(+), 8 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic43603c1b538af08fddcab1b2c1f6ad1af1a6cb9
Gerrit-Change-Number: 12823
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>

[Impala-ASF-CR] IMPALA-8330: Impala shell config file should support flag names

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

Change subject: IMPALA-8330: Impala shell config file should support flag names
......................................................................


Patch Set 7:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic43603c1b538af08fddcab1b2c1f6ad1af1a6cb9
Gerrit-Change-Number: 12823
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@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: Jim Apple <jb...@apache.org>
Gerrit-Comment-Date: Thu, 28 Mar 2019 19:16:34 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8330: Impala shell config file should use flag names

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

Change subject: IMPALA-8330: Impala shell config file should use flag names
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12823/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/12823/2//COMMIT_MSG@23
PS2, Line 23: Q
Honestly, I'm not a super big fan of this short naming. I think we'd unnecessarily make this confusing. Thoughts?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic43603c1b538af08fddcab1b2c1f6ad1af1a6cb9
Gerrit-Change-Number: 12823
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Mar 2019 19:06:42 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8330: Impala shell config file should support flag names

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

Change subject: IMPALA-8330: Impala shell config file should support flag names
......................................................................


Patch Set 6: Code-Review+1

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12823/6/shell/option_parser.py
File shell/option_parser.py:

http://gerrit.cloudera.org:8080/#/c/12823/6/shell/option_parser.py@74
PS6, Line 74:     option_dests[option.dest] = option
You might want to check that option1.dest is never equal to any option2._long_opts unless option1 == option2.


http://gerrit.cloudera.org:8080/#/c/12823/6/tests/shell/good_impalarc
File tests/shell/good_impalarc:

http://gerrit.cloudera.org:8080/#/c/12823/6/tests/shell/good_impalarc@4
PS6, Line 4: '
nit: stray character?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic43603c1b538af08fddcab1b2c1f6ad1af1a6cb9
Gerrit-Change-Number: 12823
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@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: Jim Apple <jb...@apache.org>
Gerrit-Comment-Date: Thu, 28 Mar 2019 02:00:09 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8330: Impala shell config file should support flag names

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

Change subject: IMPALA-8330: Impala shell config file should support flag names
......................................................................


Patch Set 7: Code-Review+1

(2 comments)

Carry +1 from Jim.

http://gerrit.cloudera.org:8080/#/c/12823/6/shell/option_parser.py
File shell/option_parser.py:

http://gerrit.cloudera.org:8080/#/c/12823/6/shell/option_parser.py@74
PS6, Line 74:       # Allowing dest name for backward compatibility.
> You might want to check that option1.dest is never equal to any option2._lo
Done


http://gerrit.cloudera.org:8080/#/c/12823/6/tests/shell/good_impalarc
File tests/shell/good_impalarc:

http://gerrit.cloudera.org:8080/#/c/12823/6/tests/shell/good_impalarc@4
PS6, Line 4:  
> nit: stray character?
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic43603c1b538af08fddcab1b2c1f6ad1af1a6cb9
Gerrit-Change-Number: 12823
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@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: Jim Apple <jb...@apache.org>
Gerrit-Comment-Date: Thu, 28 Mar 2019 18:45:25 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8330: Impala shell config file should support flag names

Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/12823 )

Change subject: IMPALA-8330: Impala shell config file should support flag names
......................................................................

IMPALA-8330: Impala shell config file should support flag names

This patch updates the file format in Impala shell config file to accept
both short and long flag names in addition to optparse's dest names
(variable names to store flag values) for better user experience because
dest names are internal to Impala shell.

Format:
[impala]
flag_name=flag_value

Example:
[impala]
; This is long flag.
query=select 1
; This is short flag.
Q=DEFAULT_FILE_FORMAT=parquet
; Flags can be repeated with ,
var=msg1=hello,var=msg2=world
; The old format using internal variable name is still supported for
; backward compatibility.
keyval=msg3=foo,keyval=msg4=bar

Testing:
- Ran all E2E shell tests on Python 2.6 and 2.7.

Change-Id: Ic43603c1b538af08fddcab1b2c1f6ad1af1a6cb9
---
M shell/impala_shell.py
M shell/option_parser.py
M tests/shell/good_impalarc
M tests/shell/test_shell_commandline.py
4 files changed, 45 insertions(+), 7 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic43603c1b538af08fddcab1b2c1f6ad1af1a6cb9
Gerrit-Change-Number: 12823
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@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: Jim Apple <jb...@apache.org>

[Impala-ASF-CR] IMPALA-8330: Impala shell config file should use flag names

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

Change subject: IMPALA-8330: Impala shell config file should use flag names
......................................................................


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/12823/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/12823/2//COMMIT_MSG@23
PS2, Line 23: Q
> It's just an additional feature that we support. I agree that users should 
Okay. For a second, I forgot that the options parser supported short opts. So I got confused.


http://gerrit.cloudera.org:8080/#/c/12823/2/shell/option_parser.py
File shell/option_parser.py:

http://gerrit.cloudera.org:8080/#/c/12823/2/shell/option_parser.py@67
PS2, Line 67: contains short and long options for a quick lookup.
also mention it maps to the dest?


http://gerrit.cloudera.org:8080/#/c/12823/2/shell/option_parser.py@275
PS2, Line 275:       short_opt = option._short_opts[0][1:] if len(option._short_opts) > 0 else None
I had to print these stuff locally to understand whats going on here. Add an example may be? (that it returns ['-f'] and ['--foo']...


http://gerrit.cloudera.org:8080/#/c/12823/2/shell/option_parser.py@277
PS2, Line 277:       if short_opt in defaults:
I think this part is subtle. It looks like we are updating defaults to also map from dest_name to default_val... May be add a line or two about it?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic43603c1b538af08fddcab1b2c1f6ad1af1a6cb9
Gerrit-Change-Number: 12823
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Mar 2019 22:06:43 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8330: Impala shell config file should use flag names

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

Change subject: IMPALA-8330: Impala shell config file should use flag names
......................................................................


Patch Set 3:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic43603c1b538af08fddcab1b2c1f6ad1af1a6cb9
Gerrit-Change-Number: 12823
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 22 Mar 2019 00:35:23 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8330: Impala shell config file should use flag names

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

Change subject: IMPALA-8330: Impala shell config file should use flag names
......................................................................


Patch Set 3:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/12823/2/shell/option_parser.py
File shell/option_parser.py:

http://gerrit.cloudera.org:8080/#/c/12823/2/shell/option_parser.py@67
PS2, Line 67: maps short and long option name to option for a qui
> also mention it maps to the dest?
Done. it actually maps to option. Option can have attributes like "action", "dest", etc.


http://gerrit.cloudera.org:8080/#/c/12823/2/shell/option_parser.py@275
PS2, Line 275:       # The code below removes the - from the short option and -- from the long option.
> I had to print these stuff locally to understand whats going on here. Add a
Done


http://gerrit.cloudera.org:8080/#/c/12823/2/shell/option_parser.py@277
PS2, Line 277:       long_opt = option._long_opts[0][2:] if len(option._long_opts) > 0 else None
> I think this part is subtle. It looks like we are updating defaults to also
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic43603c1b538af08fddcab1b2c1f6ad1af1a6cb9
Gerrit-Change-Number: 12823
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 22 Mar 2019 00:18:07 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8330: Impala shell config file should use flag names

Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/12823 )

Change subject: IMPALA-8330: Impala shell config file should use flag names
......................................................................

IMPALA-8330: Impala shell config file should use flag names

This patch changes the file format in Impala shell config file to accept
both short and long flag names instead of optparse's dest names
(variable names to store flag values) because dest names are internal to
Impala shell. This patch does not change the format and behavior of
Impala shell config file with [impala.query_options] section.

Format:
[impala]
flag_name=flag_value

Example:
[impala]
; This is long flag.
query=select 1
; This is short flag.
Q=DEFAULT_FILE_FORMAT=parquet
; Flags can be repeated with ,
var=msg1=hello,var=msg2=world

Testing:
- Ran all E2E shell tests on Python 2.6 and 2.7.

Change-Id: Ic43603c1b538af08fddcab1b2c1f6ad1af1a6cb9
---
M shell/impala_shell.py
M shell/option_parser.py
M tests/shell/good_impalarc
3 files changed, 31 insertions(+), 7 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic43603c1b538af08fddcab1b2c1f6ad1af1a6cb9
Gerrit-Change-Number: 12823
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-8330: Impala shell config file should use flag names

Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/12823 )

Change subject: IMPALA-8330: Impala shell config file should use flag names
......................................................................

IMPALA-8330: Impala shell config file should use flag names

This patch changes the file format in Impala shell config file to accept
both short and long flag names instead of dest names because dest names
are internal to Impala shell. This patch does not change the format and
behavior of Impala shell config file with [impala.query_options] section.

Format:
[impala]
flag_name=flag_value

Example:
[impala]
; This is long flag.
query=select 1
; This is short flag.
Q=DEFAULT_FILE_FORMAT=parquet
; Flags can be repeated.
var=msg1=hello
var=msg2=world

Testing:
- Ran all E2E shell tests

Change-Id: Ic43603c1b538af08fddcab1b2c1f6ad1af1a6cb9
---
M shell/impala_shell.py
M shell/option_parser.py
M tests/shell/good_impalarc
3 files changed, 31 insertions(+), 8 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic43603c1b538af08fddcab1b2c1f6ad1af1a6cb9
Gerrit-Change-Number: 12823
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-8330: Impala shell config file should use flag names

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

Change subject: IMPALA-8330: Impala shell config file should use flag names
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12823/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/12823/2//COMMIT_MSG@23
PS2, Line 23: Q
> Honestly, I'm not a super big fan of this short naming. I think we'd unnece
It's just an additional feature that we support. I agree that users should use long flag instead. The problem is if we ever have a flag without long name then it will become the limitation of the config file.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic43603c1b538af08fddcab1b2c1f6ad1af1a6cb9
Gerrit-Change-Number: 12823
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Mar 2019 19:11:05 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8330: Impala shell config file should support flag names

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

Change subject: IMPALA-8330: Impala shell config file should support flag names
......................................................................


Patch Set 7: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12823/5/shell/option_parser.py
File shell/option_parser.py:

http://gerrit.cloudera.org:8080/#/c/12823/5/shell/option_parser.py@132
PS5, Line 132:       print >> sys.stderr, "WARNING: Option 'config_file' can be only set from shell."
> Do we need this? we convert it to uppercase anyway (L144)
I think you missed this, do we need this?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic43603c1b538af08fddcab1b2c1f6ad1af1a6cb9
Gerrit-Change-Number: 12823
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@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: Jim Apple <jb...@apache.org>
Gerrit-Comment-Date: Thu, 28 Mar 2019 20:24:31 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8330: Impala shell config file should support flag names

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

Change subject: IMPALA-8330: Impala shell config file should support flag names
......................................................................


Patch Set 6:

TBH, the original approach wasn't usable at all and I don't mind deprecating it. I highly doubt if any serious user is relying on it. Users need to know the internal variables in the shell code to configure their rc files, ouch!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic43603c1b538af08fddcab1b2c1f6ad1af1a6cb9
Gerrit-Change-Number: 12823
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@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: Jim Apple <jb...@apache.org>
Gerrit-Comment-Date: Wed, 27 Mar 2019 19:04:24 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8330: Impala shell config file should use flag names

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

Change subject: IMPALA-8330: Impala shell config file should use flag names
......................................................................


Patch Set 5: Code-Review+1

(1 comment)

Other reviewers probably have more comments. So +1 ing this. LGTM.

http://gerrit.cloudera.org:8080/#/c/12823/5/shell/option_parser.py
File shell/option_parser.py:

http://gerrit.cloudera.org:8080/#/c/12823/5/shell/option_parser.py@132
PS5, Line 132:   config = ConfigParser.ConfigParser()
Do we need this? we convert it to uppercase anyway (L144)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic43603c1b538af08fddcab1b2c1f6ad1af1a6cb9
Gerrit-Change-Number: 12823
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@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-Comment-Date: Tue, 26 Mar 2019 23:54:38 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8330: Impala shell config file should use flag names

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

Change subject: IMPALA-8330: Impala shell config file should use flag names
......................................................................


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12823/5/tests/shell/good_impalarc
File tests/shell/good_impalarc:

PS5: 
If this breaks existing .impalarc files, I'd prefer to wait until 4.0.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic43603c1b538af08fddcab1b2c1f6ad1af1a6cb9
Gerrit-Change-Number: 12823
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@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: Jim Apple <jb...@apache.org>
Gerrit-Comment-Date: Wed, 27 Mar 2019 03:26:03 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8330: Impala shell config file should use flag names

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

Change subject: IMPALA-8330: Impala shell config file should use flag names
......................................................................


Patch Set 2:

(2 comments)

Looks good

http://gerrit.cloudera.org:8080/#/c/12823/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/12823/2//COMMIT_MSG@10
PS2, Line 10: both short and long flag names instead of dest names because dest names
My life would be easier if you explained what a 'dest name' is here.


http://gerrit.cloudera.org:8080/#/c/12823/2//COMMIT_MSG@24
PS2, Line 24: ; Flags can be repeated.
Flags can be repeated. So if I have
query_options=DEFAULT_FILE_FORMAT=parquet
Q=DEFAULT_FILE_FORMAT=orc
will it all work OK?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic43603c1b538af08fddcab1b2c1f6ad1af1a6cb9
Gerrit-Change-Number: 12823
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 25 Mar 2019 18:51:20 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8330: Impala shell config file should support flag names

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/12823 )

Change subject: IMPALA-8330: Impala shell config file should support flag names
......................................................................

IMPALA-8330: Impala shell config file should support flag names

This patch updates the file format in Impala shell config file to accept
both short and long flag names in addition to optparse's dest names
(variable names to store flag values) for better user experience because
dest names are internal to Impala shell.

Format:
[impala]
flag_name=flag_value

Example:
[impala]
; This is long flag.
query=select 1
; This is short flag.
Q=DEFAULT_FILE_FORMAT=parquet
; Flags can be repeated with ,
var=msg1=hello,var=msg2=world
; The old format using internal variable name is still supported for
; backward compatibility.
keyval=msg3=foo,keyval=msg4=bar

Testing:
- Ran all E2E shell tests on Python 2.6 and 2.7.

Change-Id: Ic43603c1b538af08fddcab1b2c1f6ad1af1a6cb9
Reviewed-on: http://gerrit.cloudera.org:8080/12823
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M shell/impala_shell.py
M shell/option_parser.py
M tests/shell/good_impalarc
M tests/shell/test_shell_commandline.py
4 files changed, 45 insertions(+), 7 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic43603c1b538af08fddcab1b2c1f6ad1af1a6cb9
Gerrit-Change-Number: 12823
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@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: Jim Apple <jb...@apache.org>

[Impala-ASF-CR] IMPALA-8330: Impala shell config file should use flag names

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

Change subject: IMPALA-8330: Impala shell config file should use flag names
......................................................................


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12823/5/tests/shell/good_impalarc
File tests/shell/good_impalarc:

PS5: 
> If this breaks existing .impalarc files, I'd prefer to wait until 4.0.
I can make this backward compatible and still support the new feature, too. I'll do that in my next PS.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic43603c1b538af08fddcab1b2c1f6ad1af1a6cb9
Gerrit-Change-Number: 12823
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@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: Jim Apple <jb...@apache.org>
Gerrit-Comment-Date: Wed, 27 Mar 2019 16:47:03 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8330: Impala shell config file should use flag names

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

Change subject: IMPALA-8330: Impala shell config file should use flag names
......................................................................


Patch Set 5:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic43603c1b538af08fddcab1b2c1f6ad1af1a6cb9
Gerrit-Change-Number: 12823
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@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-Comment-Date: Tue, 26 Mar 2019 15:13:26 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8330: Impala shell config file should support flag names

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

Change subject: IMPALA-8330: Impala shell config file should support flag names
......................................................................


Patch Set 6:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic43603c1b538af08fddcab1b2c1f6ad1af1a6cb9
Gerrit-Change-Number: 12823
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@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: Jim Apple <jb...@apache.org>
Gerrit-Comment-Date: Wed, 27 Mar 2019 18:46:05 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8330: Impala shell config file should support flag names

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

Change subject: IMPALA-8330: Impala shell config file should support flag names
......................................................................


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12823/5/shell/option_parser.py
File shell/option_parser.py:

http://gerrit.cloudera.org:8080/#/c/12823/5/shell/option_parser.py@132
PS5, Line 132:       print >> sys.stderr, "WARNING: Option 'config_file' can be only set from shell."
> I think you missed this, do we need this?
Yes, in L144, we use upper case because for impala.query_options. Case sensitivity doesn't matter for query options. However for flag names, case sensitivity matters.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic43603c1b538af08fddcab1b2c1f6ad1af1a6cb9
Gerrit-Change-Number: 12823
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@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: Jim Apple <jb...@apache.org>
Gerrit-Comment-Date: Fri, 29 Mar 2019 04:55:34 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8330: Impala shell config file should support flag names

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

Change subject: IMPALA-8330: Impala shell config file should support flag names
......................................................................


Patch Set 6:

Updated the CR to no longer be a breaking change.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic43603c1b538af08fddcab1b2c1f6ad1af1a6cb9
Gerrit-Change-Number: 12823
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@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: Jim Apple <jb...@apache.org>
Gerrit-Comment-Date: Wed, 27 Mar 2019 18:04:30 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8330: Impala shell config file should use flag names

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

Change subject: IMPALA-8330: Impala shell config file should use flag names
......................................................................


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12823/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/12823/2//COMMIT_MSG@10
PS2, Line 10: both short and long flag names instead of optparse's dest names
> My life would be easier if you explained what a 'dest name' is here.
Done


http://gerrit.cloudera.org:8080/#/c/12823/2//COMMIT_MSG@24
PS2, Line 24: Q=DEFAULT_FILE_FORMAT=parquet
> Flags can be repeated. So if I have
Impala shell allows it, e.g.

impala-shell.sh -query_option=DEFAULT=parquet -Q DEFAULT_FILE_FORMAT=orc

The behavior is undefined, it can either be parquet or orc.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic43603c1b538af08fddcab1b2c1f6ad1af1a6cb9
Gerrit-Change-Number: 12823
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 26 Mar 2019 14:40:35 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8330: Impala shell config file should support flag names

Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/12823 )

Change subject: IMPALA-8330: Impala shell config file should support flag names
......................................................................

IMPALA-8330: Impala shell config file should support flag names

This patch updates the file format in Impala shell config file to accept
both short and long flag names in addition to optparse's dest names
(variable names to store flag values) for better user experience because
dest names are internal to Impala shell.

Format:
[impala]
flag_name=flag_value

Example:
[impala]
; This is long flag.
query=select 1
; This is short flag.
Q=DEFAULT_FILE_FORMAT=parquet
; Flags can be repeated with ,
var=msg1=hello,var=msg2=world
; The old format using internal variable name is still supported for
; backward compatibility.
keyval=msg3=foo,keyval=msg4=bar

Testing:
- Ran all E2E shell tests on Python 2.6 and 2.7.

Change-Id: Ic43603c1b538af08fddcab1b2c1f6ad1af1a6cb9
---
M shell/impala_shell.py
M shell/option_parser.py
M tests/shell/good_impalarc
M tests/shell/test_shell_commandline.py
4 files changed, 44 insertions(+), 7 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic43603c1b538af08fddcab1b2c1f6ad1af1a6cb9
Gerrit-Change-Number: 12823
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@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: Jim Apple <jb...@apache.org>

[Impala-ASF-CR] IMPALA-8330: Impala shell config file should support flag names

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

Change subject: IMPALA-8330: Impala shell config file should support flag names
......................................................................


Patch Set 8: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic43603c1b538af08fddcab1b2c1f6ad1af1a6cb9
Gerrit-Change-Number: 12823
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@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: Jim Apple <jb...@apache.org>
Gerrit-Comment-Date: Fri, 29 Mar 2019 04:56:08 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8330: Impala shell config file should support flag names

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

Change subject: IMPALA-8330: Impala shell config file should support flag names
......................................................................


Patch Set 8:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic43603c1b538af08fddcab1b2c1f6ad1af1a6cb9
Gerrit-Change-Number: 12823
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@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: Jim Apple <jb...@apache.org>
Gerrit-Comment-Date: Fri, 29 Mar 2019 04:56:09 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8330: Impala shell config file should use flag names

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

Change subject: IMPALA-8330: Impala shell config file should use flag names
......................................................................


Patch Set 2:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic43603c1b538af08fddcab1b2c1f6ad1af1a6cb9
Gerrit-Change-Number: 12823
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Mar 2019 18:00:23 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8330: Impala shell config file should support flag names

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

Change subject: IMPALA-8330: Impala shell config file should support flag names
......................................................................


Patch Set 8: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic43603c1b538af08fddcab1b2c1f6ad1af1a6cb9
Gerrit-Change-Number: 12823
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@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: Jim Apple <jb...@apache.org>
Gerrit-Comment-Date: Fri, 29 Mar 2019 09:38:22 +0000
Gerrit-HasComments: No