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 <[email protected]> Gerrit-Reviewer: Anonymous Coward (395) Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Fredy Wijaya <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Comment-Date: Wed, 20 Mar 2019 21:39:17 +0000 Gerrit-HasComments: Yes
