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/18 20:51:09 UTC

[Impala-ASF-CR] IMPALA-8317: AAdd support for list type flags in Impala shell config file 

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


Change subject: IMPALA-8317: AAdd support for list type flags in Impala shell config file
......................................................................

IMPALA-8317: AAdd support for list type flags in Impala shell config file

This patch adds support for list type flags in Impala shell config
file, i.e. those that use action="append", such as --var and
--query_option. To make it less error-prone, this patch also updates
the logic for bool flags in the config file to also look at the
correct type from the argument parser instead of relying on whether or
not the default values are set in impala_shell_config_defaults.py.

Testing:
- Added a new test for list type flags
- Ran all shell E2E tests

Change-Id: I824ca15b4e1064a391b13deef9cecd34c928ef73
---
M shell/option_parser.py
M tests/shell/good_impalarc
M tests/shell/test_shell_commandline.py
3 files changed, 20 insertions(+), 4 deletions(-)



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

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

[Impala-ASF-CR] IMPALA-8317: Add support for list type flags in Impala shell config file

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

Change subject: IMPALA-8317: Add support for list type flags in Impala shell config file
......................................................................

IMPALA-8317: Add support for list type flags in Impala shell config file

This patch adds support for list type flags in Impala shell config
file, i.e. those that use action="append", such as --var and
--query_option. To make it less error-prone, this patch also updates
the logic for bool flags in the config file to also look at the
correct type from the argument parser instead of relying on whether or
not the default values are set in impala_shell_config_defaults.py.

Testing:
- Added a new test for list type flags
- Ran all shell E2E tests

Change-Id: I824ca15b4e1064a391b13deef9cecd34c928ef73
---
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, 48 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/12781/9
-- 
To view, visit http://gerrit.cloudera.org:8080/12781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I824ca15b4e1064a391b13deef9cecd34c928ef73
Gerrit-Change-Number: 12781
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (395)
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>

[Impala-ASF-CR] IMPALA-8317: Add support for list type flags in Impala shell config file

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

Change subject: IMPALA-8317: Add support for list type flags in Impala shell config file
......................................................................


Patch Set 2:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I824ca15b4e1064a391b13deef9cecd34c928ef73
Gerrit-Change-Number: 12781
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (395)
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: Mon, 18 Mar 2019 21:31:07 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8317: Add support for list type flags in Impala shell config file

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

Change subject: IMPALA-8317: Add support for list type flags in Impala shell config file
......................................................................

IMPALA-8317: Add support for list type flags in Impala shell config file

This patch adds support for list type flags in Impala shell config
file, i.e. those that use action="append", such as --var and
--query_option. To make it less error-prone, this patch also updates
the logic for bool flags in the config file to also look at the
correct type from the argument parser instead of relying on whether or
not the default values are set in impala_shell_config_defaults.py.

Testing:
- Added a new test for list type flags
- Ran all shell E2E tests

Change-Id: I824ca15b4e1064a391b13deef9cecd34c928ef73
---
M shell/impala_shell.py
M shell/option_parser.py
M tests/shell/good_impalarc
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
5 files changed, 59 insertions(+), 11 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/12781/11
-- 
To view, visit http://gerrit.cloudera.org:8080/12781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I824ca15b4e1064a391b13deef9cecd34c928ef73
Gerrit-Change-Number: 12781
Gerrit-PatchSet: 11
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (395)
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>

[Impala-ASF-CR] IMPALA-8317: Add support for list type flags in Impala shell config file

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

Change subject: IMPALA-8317: Add support for list type flags in Impala shell config file
......................................................................


Patch Set 11: Code-Review+2

Fixed minor issue. Carry +2.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I824ca15b4e1064a391b13deef9cecd34c928ef73
Gerrit-Change-Number: 12781
Gerrit-PatchSet: 11
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (395)
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: Thu, 21 Mar 2019 06:12:05 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8317: Add support for list type flags in Impala shell config file

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

Change subject: IMPALA-8317: Add support for list type flags in Impala shell config file
......................................................................


Patch Set 9: Code-Review+2

(3 comments)

Carry Bharath's +2.

http://gerrit.cloudera.org:8080/#/c/12781/8/shell/option_parser.py
File shell/option_parser.py:

http://gerrit.cloudera.org:8080/#/c/12781/8/shell/option_parser.py@78
PS8, Line 78:       result[option] = value.split('\n')
> Kinda curious why you have to split on '\n' here again. Looks like you are 
Yeah this probably has something to do with the implementation of ConfigParser. See https://stackoverflow.com/questions/15848674/how-to-configparse-a-file-keeping-multiple-values-for-identical-keys#comment91291420_15848928.


http://gerrit.cloudera.org:8080/#/c/12781/8/shell/option_parser.py@86
PS8, Line 86: class MultiOrderedDict(OrderedDict):
> Add a doc? (why this is used)
Done


http://gerrit.cloudera.org:8080/#/c/12781/8/tests/shell/good_impalarc
File tests/shell/good_impalarc:

http://gerrit.cloudera.org:8080/#/c/12781/8/tests/shell/good_impalarc@4
PS8, Line 4: keyval=msg1=hello
> nit: Move the comment to the opt_parser where we parse this?
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I824ca15b4e1064a391b13deef9cecd34c928ef73
Gerrit-Change-Number: 12781
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (395)
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: Thu, 21 Mar 2019 00:48:36 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8317: Add support for list type flags in Impala shell config file

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
Anonymous Coward (395) has posted comments on this change. ( http://gerrit.cloudera.org:8080/12781 )

Change subject: IMPALA-8317: Add support for list type flags in Impala shell config file
......................................................................


Patch Set 3: Code-Review+1

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/12781/3//COMMIT_MSG@16
PS3, Line 16: Testing:
Can you add a test to verify that the inline '--var=..'  will work compatibly with the default 'keyval' in the config_file as well?

LGTM otherwise.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I824ca15b4e1064a391b13deef9cecd34c928ef73
Gerrit-Change-Number: 12781
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (395)
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: Mon, 18 Mar 2019 21:03:21 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8317: Add support for list type flags in Impala shell config file

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

Change subject: IMPALA-8317: Add support for list type flags in Impala shell config file
......................................................................

IMPALA-8317: Add support for list type flags in Impala shell config file

This patch adds support for list type flags in Impala shell config
file, i.e. those that use action="append", such as --var and
--query_option. To make it less error-prone, this patch also updates
the logic for bool flags in the config file to also look at the
correct type from the argument parser instead of relying on whether or
not the default values are set in impala_shell_config_defaults.py.

Testing:
- Added a new test for list type flags
- Ran all shell E2E tests

Change-Id: I824ca15b4e1064a391b13deef9cecd34c928ef73
---
M shell/option_parser.py
M tests/shell/good_impalarc
M tests/shell/test_shell_commandline.py
3 files changed, 45 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/12781/8
-- 
To view, visit http://gerrit.cloudera.org:8080/12781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I824ca15b4e1064a391b13deef9cecd34c928ef73
Gerrit-Change-Number: 12781
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (395)
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>

[Impala-ASF-CR] IMPALA-8317: Add support for list type flags in Impala shell config file

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

Change subject: IMPALA-8317: Add support for list type flags in Impala shell config file
......................................................................


Patch Set 4: Code-Review+1

(1 comment)

Carry +1.

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

http://gerrit.cloudera.org:8080/#/c/12781/3//COMMIT_MSG@16
PS3, Line 16: Testing:
> Can you add a test to verify that the inline '--var=..'  will work compatib
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I824ca15b4e1064a391b13deef9cecd34c928ef73
Gerrit-Change-Number: 12781
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (395)
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: Mon, 18 Mar 2019 23:05:44 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8317: Add support for list type flags in Impala shell config file

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

Change subject: IMPALA-8317: Add support for list type flags in Impala shell config file
......................................................................


Patch Set 7:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I824ca15b4e1064a391b13deef9cecd34c928ef73
Gerrit-Change-Number: 12781
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (395)
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: Wed, 20 Mar 2019 22:23:35 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8317: Add support for list type flags in Impala shell config file

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

Change subject: IMPALA-8317: Add support for list type flags in Impala shell config file
......................................................................


Patch Set 8:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I824ca15b4e1064a391b13deef9cecd34c928ef73
Gerrit-Change-Number: 12781
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (395)
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: Wed, 20 Mar 2019 22:41:03 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8317: Add support for list type flags in Impala shell config file

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

Change subject: IMPALA-8317: Add support for list type flags in Impala shell config file
......................................................................


Patch Set 10:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I824ca15b4e1064a391b13deef9cecd34c928ef73
Gerrit-Change-Number: 12781
Gerrit-PatchSet: 10
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (395)
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: Thu, 21 Mar 2019 00:49:15 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8317: Add support for list type flags in Impala shell config file

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

Change subject: IMPALA-8317: Add support for list type flags in Impala shell config file
......................................................................


Patch Set 4:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I824ca15b4e1064a391b13deef9cecd34c928ef73
Gerrit-Change-Number: 12781
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (395)
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: Mon, 18 Mar 2019 23:50:32 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8317: Add support for list type flags in Impala shell config file

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

Change subject: IMPALA-8317: Add support for list type flags in Impala shell config file
......................................................................


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12781/4/shell/option_parser.py
File shell/option_parser.py:

http://gerrit.cloudera.org:8080/#/c/12781/4/shell/option_parser.py@77
PS4, Line 77: value.split(",")
Not super sure if this is the right way. Why always a ','?

I'd assume the way it works with append stuff is that you need to provide each option separately.

For ex: --var=foo=bar --var=x=y ....

Shouldn't we keep that behavior consistent with the rc files here?

For example, the above should translate to

[impala]
...
keyval=foo=bar
keyval=x=y

Thoughts?


http://gerrit.cloudera.org:8080/#/c/12781/4/tests/shell/good_impalarc
File tests/shell/good_impalarc:

http://gerrit.cloudera.org:8080/#/c/12781/4/tests/shell/good_impalarc@4
PS4, Line 4: keyval
I'm a little bit surprised by this. I'd assume these should map to the actual shell option rather than the "dest" string.  How'd the customer know the "dest" variables here without having to look at the shell code? Thoughts?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I824ca15b4e1064a391b13deef9cecd34c928ef73
Gerrit-Change-Number: 12781
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (395)
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: Wed, 20 Mar 2019 20:40:56 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8317: Add support for list type flags in Impala shell config file

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

Change subject: IMPALA-8317: Add support for list type flags in Impala shell config file
......................................................................

IMPALA-8317: Add support for list type flags in Impala shell config file

This patch adds support for list type flags in Impala shell config
file, i.e. those that use action="append", such as --var and
--query_option. To make it less error-prone, this patch also updates
the logic for bool flags in the config file to also look at the
correct type from the argument parser instead of relying on whether or
not the default values are set in impala_shell_config_defaults.py.

Testing:
- Added a new test for list type flags
- Ran all shell E2E tests

Change-Id: I824ca15b4e1064a391b13deef9cecd34c928ef73
Reviewed-on: http://gerrit.cloudera.org:8080/12781
Reviewed-by: Fredy Wijaya <fw...@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
M tests/shell/test_shell_interactive.py
5 files changed, 59 insertions(+), 11 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I824ca15b4e1064a391b13deef9cecd34c928ef73
Gerrit-Change-Number: 12781
Gerrit-PatchSet: 12
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (395)
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>

[Impala-ASF-CR] IMPALA-8317: Add support for list type flags in Impala shell config file

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

Change subject: IMPALA-8317: Add support for list type flags in Impala shell config file
......................................................................


Patch Set 9:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I824ca15b4e1064a391b13deef9cecd34c928ef73
Gerrit-Change-Number: 12781
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (395)
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: Thu, 21 Mar 2019 01:32:48 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8317: Add support for list type flags in Impala shell config file

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

Change subject: IMPALA-8317: Add support for list type flags in Impala shell config file
......................................................................


Patch Set 11: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I824ca15b4e1064a391b13deef9cecd34c928ef73
Gerrit-Change-Number: 12781
Gerrit-PatchSet: 11
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (395)
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: Thu, 21 Mar 2019 10:29:42 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8317: Add support for list type flags in Impala shell config file

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

Change subject: IMPALA-8317: Add support for list type flags in Impala shell config file
......................................................................

IMPALA-8317: Add support for list type flags in Impala shell config file

This patch adds support for list type flags in Impala shell config
file, i.e. those that use action="append", such as --var and
--query_option. To make it less error-prone, this patch also updates
the logic for bool flags in the config file to also look at the
correct type from the argument parser instead of relying on whether or
not the default values are set in impala_shell_config_defaults.py.

Testing:
- Added a new test for list type flags
- Ran all shell E2E tests

Change-Id: I824ca15b4e1064a391b13deef9cecd34c928ef73
---
M shell/option_parser.py
M tests/shell/good_impalarc
M tests/shell/test_shell_commandline.py
3 files changed, 44 insertions(+), 8 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I824ca15b4e1064a391b13deef9cecd34c928ef73
Gerrit-Change-Number: 12781
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (395)
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>

[Impala-ASF-CR] IMPALA-8317: Add support for list type flags in Impala shell config file

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

Change subject: IMPALA-8317: Add support for list type flags in Impala shell config file
......................................................................

IMPALA-8317: Add support for list type flags in Impala shell config file

This patch adds support for list type flags in Impala shell config
file, i.e. those that use action="append", such as --var and
--query_option. To make it less error-prone, this patch also updates
the logic for bool flags in the config file to also look at the
correct type from the argument parser instead of relying on whether or
not the default values are set in impala_shell_config_defaults.py.

Testing:
- Added a new test for list type flags
- Ran all shell E2E tests

Change-Id: I824ca15b4e1064a391b13deef9cecd34c928ef73
---
M shell/option_parser.py
M tests/shell/good_impalarc
M tests/shell/test_shell_commandline.py
3 files changed, 20 insertions(+), 4 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I824ca15b4e1064a391b13deef9cecd34c928ef73
Gerrit-Change-Number: 12781
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-8317: Add support for list type flags in Impala shell config file

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

Change subject: IMPALA-8317: Add support for list type flags in Impala shell config file
......................................................................


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12781/4/shell/option_parser.py
File shell/option_parser.py:

http://gerrit.cloudera.org:8080/#/c/12781/4/shell/option_parser.py@77
PS4, Line 77: value.split(",")
> Fair point. The reason I did was because we used ConfigParser which doesn't
Do we need this fancy MultiOrderedDict here? Can't we just do

elif opt.action == append:
  if not option in result.keys():
    result[option] = []
  result[option].append(value)

Does that not work for some reason?


http://gerrit.cloudera.org:8080/#/c/12781/4/tests/shell/good_impalarc
File tests/shell/good_impalarc:

http://gerrit.cloudera.org:8080/#/c/12781/4/tests/shell/good_impalarc@4
PS4, Line 4: keyval
> Yeah, Vincent and I were talking about the same concern here. I agree it's 
Thanks, mind adding a TODO there with the jira reference? (so that others don't get confused when they see it for the first time, like me :-))



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I824ca15b4e1064a391b13deef9cecd34c928ef73
Gerrit-Change-Number: 12781
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (395)
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: Wed, 20 Mar 2019 22:08:10 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8317: Add support for list type flags in Impala shell config file

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

Change subject: IMPALA-8317: Add support for list type flags in Impala shell config file
......................................................................

IMPALA-8317: Add support for list type flags in Impala shell config file

This patch adds support for list type flags in Impala shell config
file, i.e. those that use action="append", such as --var and
--query_option. To make it less error-prone, this patch also updates
the logic for bool flags in the config file to also look at the
correct type from the argument parser instead of relying on whether or
not the default values are set in impala_shell_config_defaults.py.

Testing:
- Added a new test for list type flags
- Ran all shell E2E tests

Change-Id: I824ca15b4e1064a391b13deef9cecd34c928ef73
---
M shell/option_parser.py
M tests/shell/good_impalarc
M tests/shell/test_shell_commandline.py
3 files changed, 28 insertions(+), 4 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I824ca15b4e1064a391b13deef9cecd34c928ef73
Gerrit-Change-Number: 12781
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (395)
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-8317: Add support for list type flags in Impala shell config file

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

Change subject: IMPALA-8317: Add support for list type flags in Impala shell config file
......................................................................


Patch Set 7:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12781/4/shell/option_parser.py
File shell/option_parser.py:

http://gerrit.cloudera.org:8080/#/c/12781/4/shell/option_parser.py@77
PS4, Line 77: "append":
> Not super sure if this is the right way. Why always a ','?
Fair point. The reason I did was because we used ConfigParser which doesn't allow duplicate keys by default. There's a way to customize it, but not as flexible as the one in Python 3. Anyway I updated the CR to fix this. The split is still necessary because in Python 2.6 and 2.7, the duplicate keys will be appended with \n. Done.


http://gerrit.cloudera.org:8080/#/c/12781/4/tests/shell/good_impalarc
File tests/shell/good_impalarc:

http://gerrit.cloudera.org:8080/#/c/12781/4/tests/shell/good_impalarc@4
PS4, Line 4: keyval
> I'm a little bit surprised by this. I'd assume these should map to the actu
Yeah, Vincent and I were talking about the same concern here. I agree it's also super confusing and we should use the flag name and not the dest name. Unfortunately it's been like that since the beginning. We definitely should fix this though. I filed a JIRA here: https://issues.apache.org/jira/browse/IMPALA-8330



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I824ca15b4e1064a391b13deef9cecd34c928ef73
Gerrit-Change-Number: 12781
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (395)
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: Wed, 20 Mar 2019 21:39:17 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8317: Add support for list type flags in Impala shell config file

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

Change subject: IMPALA-8317: Add support for list type flags in Impala shell config file
......................................................................


Patch Set 10: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I824ca15b4e1064a391b13deef9cecd34c928ef73
Gerrit-Change-Number: 12781
Gerrit-PatchSet: 10
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (395)
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: Thu, 21 Mar 2019 00:49:14 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8317: Add support for list type flags in Impala shell config file

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

Change subject: IMPALA-8317: Add support for list type flags in Impala shell config file
......................................................................


Patch Set 8: Code-Review+2

(5 comments)

http://gerrit.cloudera.org:8080/#/c/12781/4/shell/option_parser.py
File shell/option_parser.py:

http://gerrit.cloudera.org:8080/#/c/12781/4/shell/option_parser.py@77
PS4, Line 77: "append":
> Yeah because we parse the INI file using the parser. So the "options" varia
Ah gotcha


http://gerrit.cloudera.org:8080/#/c/12781/8/shell/option_parser.py
File shell/option_parser.py:

http://gerrit.cloudera.org:8080/#/c/12781/8/shell/option_parser.py@78
PS8, Line 78:       result[option] = value.split('\n')
Kinda curious why you have to split on '\n' here again. Looks like you are already extend() ing in the __set_item__ override below, so I'd guess value here should be a list? Haven't dug into it but curious why.


http://gerrit.cloudera.org:8080/#/c/12781/8/shell/option_parser.py@86
PS8, Line 86: class MultiOrderedDict(OrderedDict):
Add a doc? (why this is used)


http://gerrit.cloudera.org:8080/#/c/12781/8/tests/shell/good_impalarc
File tests/shell/good_impalarc:

http://gerrit.cloudera.org:8080/#/c/12781/8/tests/shell/good_impalarc@4
PS8, Line 4: # TODO: IMPALA-8330: Use flag names instead of dest names.
nit: Move the comment to the opt_parser where we parse this?


http://gerrit.cloudera.org:8080/#/c/12781/8/tests/shell/good_impalarc@9
PS8, Line 9: 
May be also add another keyval in the end (to make sure it gets parsed too)?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I824ca15b4e1064a391b13deef9cecd34c928ef73
Gerrit-Change-Number: 12781
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (395)
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: Thu, 21 Mar 2019 00:22:40 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8317: Add support for list type flags in Impala shell config file

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

Change subject: IMPALA-8317: Add support for list type flags in Impala shell config file
......................................................................


Patch Set 10: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3930/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I824ca15b4e1064a391b13deef9cecd34c928ef73
Gerrit-Change-Number: 12781
Gerrit-PatchSet: 10
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (395)
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: Thu, 21 Mar 2019 05:14:30 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8317: Add support for list type flags in Impala shell config file

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

Change subject: IMPALA-8317: Add support for list type flags in Impala shell config file
......................................................................


Patch Set 8:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12781/4/shell/option_parser.py
File shell/option_parser.py:

http://gerrit.cloudera.org:8080/#/c/12781/4/shell/option_parser.py@77
PS4, Line 77: "append":
> Do we need this fancy MultiOrderedDict here? Can't we just do
Yeah because we parse the INI file using the parser. So the "options" variable is what comes from the parser and without customizing the parser, it will not handle duplicate keys.


http://gerrit.cloudera.org:8080/#/c/12781/4/tests/shell/good_impalarc
File tests/shell/good_impalarc:

http://gerrit.cloudera.org:8080/#/c/12781/4/tests/shell/good_impalarc@4
PS4, Line 4: # TODO
> Thanks, mind adding a TODO there with the jira reference? (so that others d
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I824ca15b4e1064a391b13deef9cecd34c928ef73
Gerrit-Change-Number: 12781
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (395)
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: Wed, 20 Mar 2019 22:15:36 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8317: Add support for list type flags in Impala shell config file

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

Change subject: IMPALA-8317: Add support for list type flags in Impala shell config file
......................................................................


Patch Set 3:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I824ca15b4e1064a391b13deef9cecd34c928ef73
Gerrit-Change-Number: 12781
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (395)
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: Mon, 18 Mar 2019 21:40:05 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8317: Add support for list type flags in Impala shell config file

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

Change subject: IMPALA-8317: Add support for list type flags in Impala shell config file
......................................................................


Patch Set 11:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I824ca15b4e1064a391b13deef9cecd34c928ef73
Gerrit-Change-Number: 12781
Gerrit-PatchSet: 11
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (395)
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: Thu, 21 Mar 2019 06:38:36 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8317: Add support for list type flags in Impala shell config file

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

Change subject: IMPALA-8317: Add support for list type flags in Impala shell config file
......................................................................


Patch Set 11:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I824ca15b4e1064a391b13deef9cecd34c928ef73
Gerrit-Change-Number: 12781
Gerrit-PatchSet: 11
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (395)
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: Thu, 21 Mar 2019 06:12:23 +0000
Gerrit-HasComments: No