You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2019/04/04 05:50:54 UTC

[Impala-ASF-CR] Clean up generation of XML configuration files 

Hello Vihang Karajgaonkar,

I'd like you to do a code review. Please visit

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

to review the following change.


Change subject: Clean up generation of XML configuration files
......................................................................

Clean up generation of XML configuration files

hive-site.xml and sentry-site.xml in particular had grown multiple
slightly-different variants, differing only in a few small pieces. This
was difficult to maintain: in fact, while attempting to clean them up I
found a number of places that the MySQL and Postgres versions of
hive-site had diverged for no apparent reason.

This moves away from using the sed-based templating for these
configuration files, and instead uses python as a poor man's template
system. That enables much simpler conditional logic.

I briefly considered XSLT for this, but decided that Python is probably
easier for the average developer to follow, modify, and debug.

Along the way, I removed a few flags which appear to be no longer used
by Hive 2 or later, and a few items which were already commented out in
the previous template.

Change-Id: Ief4434d80baae0fd7be7ffe7b2e07bae1ac45e47
---
M bin/create-test-configuration.sh
A bin/generate_xml_config.py
A fe/src/test/resources/hive-site.xml.py
D fe/src/test/resources/mysql-hive-site.xml.template
D fe/src/test/resources/postgresql-hive-site.xml.cdp.template
D fe/src/test/resources/postgresql-hive-site.xml.template
A fe/src/test/resources/sentry-site.xml.py
D fe/src/test/resources/sentry-site.xml.template
D fe/src/test/resources/sentry-site_no_oo.xml.template
D fe/src/test/resources/sentry-site_oo.xml.template
D fe/src/test/resources/sentry-site_oo_nogrant.xml.template
11 files changed, 280 insertions(+), 997 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ief4434d80baae0fd7be7ffe7b2e07bae1ac45e47
Gerrit-Change-Number: 12930
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>

[Impala-ASF-CR] Clean up generation of XML configuration files

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

Change subject: Clean up generation of XML configuration files
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12930/2/fe/src/test/resources/sentry-site.xml.py
File fe/src/test/resources/sentry-site.xml.py:

http://gerrit.cloudera.org:8080/#/c/12930/2/fe/src/test/resources/sentry-site.xml.py@49
PS2, Line 49:   'sentry.store.jdbc.url': 'jdbc:postgresql://localhost:5432/${SENTRY_POLICY_DB}/;create=true'
we should fix this JDBC URL: https://gerrit.cloudera.org/c/12894/3/fe/src/test/resources/sentry-site.xml.template



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ief4434d80baae0fd7be7ffe7b2e07bae1ac45e47
Gerrit-Change-Number: 12930
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Apr 2019 19:06:59 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] Clean up generation of XML configuration files

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

Change subject: Clean up generation of XML configuration files
......................................................................


Patch Set 2:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ief4434d80baae0fd7be7ffe7b2e07bae1ac45e47
Gerrit-Change-Number: 12930
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Apr 2019 16:52:43 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] Clean up generation of XML configuration files

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

Change subject: Clean up generation of XML configuration files
......................................................................


Patch Set 3: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ief4434d80baae0fd7be7ffe7b2e07bae1ac45e47
Gerrit-Change-Number: 12930
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Apr 2019 20:03:31 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] Clean up generation of XML configuration files

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

Change subject: Clean up generation of XML configuration files
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12930/3/bin/jenkins/critique-gerrit-review.py
File bin/jenkins/critique-gerrit-review.py:

http://gerrit.cloudera.org:8080/#/c/12930/3/bin/jenkins/critique-gerrit-review.py@72
PS3, Line 72:  
> flake8: E261 at least two spaces before inline comment
I'm tempted to disable this warning. I guess it's part of PEP-8 and therefore standard but I find it very hard to see how it makes anything more readable: https://www.python.org/dev/peps/pep-0008/#inline-comments



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ief4434d80baae0fd7be7ffe7b2e07bae1ac45e47
Gerrit-Change-Number: 12930
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Apr 2019 21:40:25 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] Clean up generation of XML configuration files

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

Change subject: Clean up generation of XML configuration files
......................................................................


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ief4434d80baae0fd7be7ffe7b2e07bae1ac45e47
Gerrit-Change-Number: 12930
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Apr 2019 21:38:51 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] Clean up generation of XML configuration files

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

Change subject: Clean up generation of XML configuration files
......................................................................


Patch Set 2:

(5 comments)

Ok no worries, yeah that makes sense. You could add the file to the list in bin/jenkins/critique-gerrit-review.py, I have no issue with that if it's problematic.

Overall this looks good, mainly just some questions about comments.

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

http://gerrit.cloudera.org:8080/#/c/12930/2//COMMIT_MSG@24
PS2, Line 24: the previous template.
Could you list the flags out? Just to make it easier to understand. I did a manual comparison and got:

* hive.stats.dbclass
* hive.metastore.rawstore.impl


http://gerrit.cloudera.org:8080/#/c/12930/2//COMMIT_MSG@25
PS2, Line 25: 
We should make sure to test on CentOS 6 since that still has python 2.6, just in case there's something incompatible in the new Python code. :sadpanda:.


http://gerrit.cloudera.org:8080/#/c/12930/2/bin/create-test-configuration.sh
File bin/create-test-configuration.sh:

http://gerrit.cloudera.org:8080/#/c/12930/2/bin/create-test-configuration.sh@32
PS2, Line 32: function generate_config {
It would be good to leave a comment here or below with thoughts about why generate_config would be used over the python-based templating (or if we want to port everything to python-based templating eventually).

Otherwise people will be confused about why two mechanisms exist.


http://gerrit.cloudera.org:8080/#/c/12930/2/bin/generate_xml_config.py
File bin/generate_xml_config.py:

http://gerrit.cloudera.org:8080/#/c/12930/2/bin/generate_xml_config.py@80
PS2, Line 80:       """ % dict(source_path=os.path.abspath(source_path))
New-style formatting - .format(source_path=...) and {source_path} is generally preferred in new code over %. Not a hard requirement but I think it should be easy to change over here and elsewhere.


http://gerrit.cloudera.org:8080/#/c/12930/2/fe/src/test/resources/hive-site.xml.py
File fe/src/test/resources/hive-site.xml.py:

http://gerrit.cloudera.org:8080/#/c/12930/2/fe/src/test/resources/hive-site.xml.py@69
PS2, Line 69: HBSE
I'm so confused by this abbreviation - why omit the A?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ief4434d80baae0fd7be7ffe7b2e07bae1ac45e47
Gerrit-Change-Number: 12930
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Apr 2019 17:36:06 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] Clean up generation of XML configuration files

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

Change subject: Clean up generation of XML configuration files
......................................................................


Patch Set 2:

If you want, you can prefetch the bot comments with

  ./bin/jenkins/critique-gerrit-review.py --dryrun


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ief4434d80baae0fd7be7ffe7b2e07bae1ac45e47
Gerrit-Change-Number: 12930
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Apr 2019 17:11:59 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] Clean up generation of XML configuration files

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

Change subject: Clean up generation of XML configuration files
......................................................................


Patch Set 3:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/12930/3/bin/jenkins/critique-gerrit-review.py
File bin/jenkins/critique-gerrit-review.py:

http://gerrit.cloudera.org:8080/#/c/12930/3/bin/jenkins/critique-gerrit-review.py@72
PS3, Line 72:  
flake8: E261 at least two spaces before inline comment


http://gerrit.cloudera.org:8080/#/c/12930/3/fe/src/test/resources/hive-site.xml.py
File fe/src/test/resources/hive-site.xml.py:

http://gerrit.cloudera.org:8080/#/c/12930/3/fe/src/test/resources/hive-site.xml.py@58
PS3, Line 58: 2
flake8: E501 line too long (96 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/12930/3/fe/src/test/resources/hive-site.xml.py@67
PS3, Line 67: e
flake8: E501 line too long (94 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/12930/3/fe/src/test/resources/hive-site.xml.py@82
PS3, Line 82: i
flake8: E501 line too long (119 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/12930/3/fe/src/test/resources/hive-site.xml.py@92
PS3, Line 92: f
flake8: E501 line too long (108 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/12930/3/fe/src/test/resources/hive-site.xml.py@93
PS3, Line 93: .
flake8: E501 line too long (117 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/12930/3/fe/src/test/resources/hive-site.xml.py@113
PS3, Line 113: t
flake8: E501 line too long (112 > 90 characters)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ief4434d80baae0fd7be7ffe7b2e07bae1ac45e47
Gerrit-Change-Number: 12930
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Apr 2019 19:31:40 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] Clean up generation of XML configuration files

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

Change subject: Clean up generation of XML configuration files
......................................................................


Patch Set 2:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/12930/2/fe/src/test/resources/hive-site.xml.py
File fe/src/test/resources/hive-site.xml.py:

http://gerrit.cloudera.org:8080/#/c/12930/2/fe/src/test/resources/hive-site.xml.py@58
PS2, Line 58: 2
flake8: E501 line too long (96 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/12930/2/fe/src/test/resources/hive-site.xml.py@67
PS2, Line 67: e
flake8: E501 line too long (94 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/12930/2/fe/src/test/resources/hive-site.xml.py@82
PS2, Line 82: i
flake8: E501 line too long (119 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/12930/2/fe/src/test/resources/hive-site.xml.py@92
PS2, Line 92: f
flake8: E501 line too long (108 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/12930/2/fe/src/test/resources/hive-site.xml.py@93
PS2, Line 93: .
flake8: E501 line too long (117 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/12930/2/fe/src/test/resources/hive-site.xml.py@113
PS2, Line 113: t
flake8: E501 line too long (112 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/12930/2/fe/src/test/resources/sentry-site.xml.py
File fe/src/test/resources/sentry-site.xml.py:

http://gerrit.cloudera.org:8080/#/c/12930/2/fe/src/test/resources/sentry-site.xml.py@49
PS2, Line 49: r
flake8: E501 line too long (95 > 90 characters)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ief4434d80baae0fd7be7ffe7b2e07bae1ac45e47
Gerrit-Change-Number: 12930
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Apr 2019 16:19:11 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] Clean up generation of XML configuration files

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

Change subject: Clean up generation of XML configuration files
......................................................................


Patch Set 3:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ief4434d80baae0fd7be7ffe7b2e07bae1ac45e47
Gerrit-Change-Number: 12930
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Apr 2019 19:56:50 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] Clean up generation of XML configuration files

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

Change subject: Clean up generation of XML configuration files
......................................................................


Patch Set 3: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ief4434d80baae0fd7be7ffe7b2e07bae1ac45e47
Gerrit-Change-Number: 12930
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Fri, 05 Apr 2019 21:30:18 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] Clean up generation of XML configuration files

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

Change subject: Clean up generation of XML configuration files
......................................................................


Patch Set 2:

Thanks. I think for this case the "long lines" are probably a good idea for readability vs lots of weird breaking of long strings. I spent some time trying to see if I could disable the "long line" warning on flake8 for a single file, but just ended up finding some bikeshed github issue arguments about whether that's a good feature or not... and I gave up. Looks like we don't have a standard of "flake8 clean" so leaving these for now.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ief4434d80baae0fd7be7ffe7b2e07bae1ac45e47
Gerrit-Change-Number: 12930
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Apr 2019 17:29:28 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] Clean up generation of XML configuration files

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

Change subject: Clean up generation of XML configuration files
......................................................................


Patch Set 2:

(7 comments)

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

http://gerrit.cloudera.org:8080/#/c/12930/2//COMMIT_MSG@24
PS2, Line 24: the previous template.
> Could you list the flags out? Just to make it easier to understand. I did a
Done


http://gerrit.cloudera.org:8080/#/c/12930/2//COMMIT_MSG@25
PS2, Line 25: 
> We should make sure to test on CentOS 6 since that still has python 2.6, ju
tested it on an el6 box and it appears to work.


http://gerrit.cloudera.org:8080/#/c/12930/2/bin/create-test-configuration.sh
File bin/create-test-configuration.sh:

http://gerrit.cloudera.org:8080/#/c/12930/2/bin/create-test-configuration.sh@32
PS2, Line 32: function generate_config {
> It would be good to leave a comment here or below with thoughts about why g
Done. Added a TODO for myself to convert over the remaining -site.xml files (hbase, ranger-admin)


http://gerrit.cloudera.org:8080/#/c/12930/2/bin/generate_xml_config.py
File bin/generate_xml_config.py:

http://gerrit.cloudera.org:8080/#/c/12930/2/bin/generate_xml_config.py@80
PS2, Line 80:       """ % dict(source_path=os.path.abspath(source_path))
> New-style formatting - .format(source_path=...) and {source_path} is genera
Done


http://gerrit.cloudera.org:8080/#/c/12930/2/bin/generate_xml_config.py@113
PS2, Line 113:       os.unlink(tmp_path)
> Does this delete the tmp file as well?
yea, unlink = delete


http://gerrit.cloudera.org:8080/#/c/12930/2/fe/src/test/resources/hive-site.xml.py
File fe/src/test/resources/hive-site.xml.py:

http://gerrit.cloudera.org:8080/#/c/12930/2/fe/src/test/resources/hive-site.xml.py@69
PS2, Line 69: HBSE
> (you don't need to fix it btw, just couldn't resist commenting)
yea I actually wasted a few minutes because I accidentally fixed the typo and then some stuff broke :) It's set with this typo elsewhere.


http://gerrit.cloudera.org:8080/#/c/12930/2/fe/src/test/resources/sentry-site.xml.py
File fe/src/test/resources/sentry-site.xml.py:

http://gerrit.cloudera.org:8080/#/c/12930/2/fe/src/test/resources/sentry-site.xml.py@49
PS2, Line 49:   'sentry.store.jdbc.url': 'jdbc:postgresql://localhost:5432/${SENTRY_POLICY_DB}/;create=true'
> we should fix this JDBC URL: https://gerrit.cloudera.org/c/12894/3/fe/src/t
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ief4434d80baae0fd7be7ffe7b2e07bae1ac45e47
Gerrit-Change-Number: 12930
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Apr 2019 19:30:49 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] Clean up generation of XML configuration files

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

Change subject: Clean up generation of XML configuration files
......................................................................


Patch Set 3:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ief4434d80baae0fd7be7ffe7b2e07bae1ac45e47
Gerrit-Change-Number: 12930
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Fri, 05 Apr 2019 16:45:43 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] Clean up generation of XML configuration files

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Vihang Karajgaonkar, Fredy Wijaya, Tim Armstrong, Impala Public Jenkins, Adam Holley, 

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

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

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

Change subject: Clean up generation of XML configuration files
......................................................................

Clean up generation of XML configuration files

hive-site.xml and sentry-site.xml in particular had grown multiple
slightly-different variants, differing only in a few small pieces. This
was difficult to maintain: in fact, while attempting to clean them up I
found a number of places that the MySQL and Postgres versions of
hive-site had diverged for no apparent reason.

This moves away from using the sed-based templating for these
configuration files, and instead uses python as a poor man's template
system. That enables much simpler conditional logic.

I briefly considered XSLT for this, but decided that Python is probably
easier for the average developer to follow, modify, and debug.

Along the way, I removed a few flags which appear to be no longer used
by Hive 2 or later, and a few items which were already commented out in
the previous template:

- hive.stats.dbclass
- hive.stats.dbconnectionstring
- hive.stats.jdbcdriver

These are no longer relevant after HIVE-12164 ("Remove jdbc stats
collection mechanism") in Hive 2.0.

- hive.metastore.rawstore.impl

This has always defaulted to 'ObjectStore' in Hive, so there was no
reason to set it explicitly.

- test.log.dir
- test.src.dir

These were listed in the config in a commented-out section. These were
commented out ever since 2012 when the file was first introduced.

This also fixes the postgres URL to not include a misplaced ';create'
parameter (which applies to Derby but not postgres).

Change-Id: Ief4434d80baae0fd7be7ffe7b2e07bae1ac45e47
---
M bin/create-test-configuration.sh
A bin/generate_xml_config.py
M bin/jenkins/critique-gerrit-review.py
A fe/src/test/resources/hive-site.xml.py
D fe/src/test/resources/mysql-hive-site.xml.template
D fe/src/test/resources/postgresql-hive-site.xml.cdp.template
D fe/src/test/resources/postgresql-hive-site.xml.template
A fe/src/test/resources/sentry-site.xml.py
D fe/src/test/resources/sentry-site.xml.template
D fe/src/test/resources/sentry-site_no_oo.xml.template
D fe/src/test/resources/sentry-site_oo.xml.template
D fe/src/test/resources/sentry-site_oo_nogrant.xml.template
12 files changed, 321 insertions(+), 998 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ief4434d80baae0fd7be7ffe7b2e07bae1ac45e47
Gerrit-Change-Number: 12930
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>

[Impala-ASF-CR] Clean up generation of XML configuration files

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

Change subject: Clean up generation of XML configuration files
......................................................................


Patch Set 2: Code-Review+1

(1 comment)

Just a question related to cleanup on errors. Rest looks good to me. Thanks for the patch. I think its a lot cleaner now.

http://gerrit.cloudera.org:8080/#/c/12930/2/bin/generate_xml_config.py
File bin/generate_xml_config.py:

http://gerrit.cloudera.org:8080/#/c/12930/2/bin/generate_xml_config.py@113
PS2, Line 113:       os.unlink(tmp_path)
Does this delete the tmp file as well?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ief4434d80baae0fd7be7ffe7b2e07bae1ac45e47
Gerrit-Change-Number: 12930
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Apr 2019 17:49:40 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] Clean up generation of XML configuration files

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12930 )

Change subject: Clean up generation of XML configuration files
......................................................................

Clean up generation of XML configuration files

hive-site.xml and sentry-site.xml in particular had grown multiple
slightly-different variants, differing only in a few small pieces. This
was difficult to maintain: in fact, while attempting to clean them up I
found a number of places that the MySQL and Postgres versions of
hive-site had diverged for no apparent reason.

This moves away from using the sed-based templating for these
configuration files, and instead uses python as a poor man's template
system. That enables much simpler conditional logic.

I briefly considered XSLT for this, but decided that Python is probably
easier for the average developer to follow, modify, and debug.

Along the way, I removed a few flags which appear to be no longer used
by Hive 2 or later, and a few items which were already commented out in
the previous template:

- hive.stats.dbclass
- hive.stats.dbconnectionstring
- hive.stats.jdbcdriver

These are no longer relevant after HIVE-12164 ("Remove jdbc stats
collection mechanism") in Hive 2.0.

- hive.metastore.rawstore.impl

This has always defaulted to 'ObjectStore' in Hive, so there was no
reason to set it explicitly.

- test.log.dir
- test.src.dir

These were listed in the config in a commented-out section. These were
commented out ever since 2012 when the file was first introduced.

This also fixes the postgres URL to not include a misplaced ';create'
parameter (which applies to Derby but not postgres).

Change-Id: Ief4434d80baae0fd7be7ffe7b2e07bae1ac45e47
Reviewed-on: http://gerrit.cloudera.org:8080/12930
Reviewed-by: Fredy Wijaya <fw...@cloudera.com>
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M bin/create-test-configuration.sh
A bin/generate_xml_config.py
M bin/jenkins/critique-gerrit-review.py
A fe/src/test/resources/hive-site.xml.py
D fe/src/test/resources/mysql-hive-site.xml.template
D fe/src/test/resources/postgresql-hive-site.xml.cdp.template
D fe/src/test/resources/postgresql-hive-site.xml.template
A fe/src/test/resources/sentry-site.xml.py
D fe/src/test/resources/sentry-site.xml.template
D fe/src/test/resources/sentry-site_no_oo.xml.template
D fe/src/test/resources/sentry-site_oo.xml.template
D fe/src/test/resources/sentry-site_oo_nogrant.xml.template
12 files changed, 321 insertions(+), 998 deletions(-)

Approvals:
  Fredy Wijaya: Looks good to me, but someone else must approve
  Tim Armstrong: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ief4434d80baae0fd7be7ffe7b2e07bae1ac45e47
Gerrit-Change-Number: 12930
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>

[Impala-ASF-CR] Clean up generation of XML configuration files

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

Change subject: Clean up generation of XML configuration files
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12930/2/fe/src/test/resources/hive-site.xml.py
File fe/src/test/resources/hive-site.xml.py:

http://gerrit.cloudera.org:8080/#/c/12930/2/fe/src/test/resources/hive-site.xml.py@69
PS2, Line 69: HBSE
> I'm so confused by this abbreviation - why omit the A?
(you don't need to fix it btw, just couldn't resist commenting)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ief4434d80baae0fd7be7ffe7b2e07bae1ac45e47
Gerrit-Change-Number: 12930
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Apr 2019 17:44:23 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] Clean up generation of XML configuration files

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

Change subject: Clean up generation of XML configuration files
......................................................................


Patch Set 1:

Build Failed 

https://jenkins.impala.io/job/gerrit-code-review-checks/2637/ : Initial code review checks failed. See linked job for details on the failure.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ief4434d80baae0fd7be7ffe7b2e07bae1ac45e47
Gerrit-Change-Number: 12930
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Apr 2019 06:48:08 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] Clean up generation of XML configuration files

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Vihang Karajgaonkar, Fredy Wijaya, Tim Armstrong, Impala Public Jenkins, Adam Holley, 

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

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

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

Change subject: Clean up generation of XML configuration files
......................................................................

Clean up generation of XML configuration files

hive-site.xml and sentry-site.xml in particular had grown multiple
slightly-different variants, differing only in a few small pieces. This
was difficult to maintain: in fact, while attempting to clean them up I
found a number of places that the MySQL and Postgres versions of
hive-site had diverged for no apparent reason.

This moves away from using the sed-based templating for these
configuration files, and instead uses python as a poor man's template
system. That enables much simpler conditional logic.

I briefly considered XSLT for this, but decided that Python is probably
easier for the average developer to follow, modify, and debug.

Along the way, I removed a few flags which appear to be no longer used
by Hive 2 or later, and a few items which were already commented out in
the previous template.

Change-Id: Ief4434d80baae0fd7be7ffe7b2e07bae1ac45e47
---
M bin/create-test-configuration.sh
A bin/generate_xml_config.py
A fe/src/test/resources/hive-site.xml.py
D fe/src/test/resources/mysql-hive-site.xml.template
D fe/src/test/resources/postgresql-hive-site.xml.cdp.template
D fe/src/test/resources/postgresql-hive-site.xml.template
A fe/src/test/resources/sentry-site.xml.py
D fe/src/test/resources/sentry-site.xml.template
D fe/src/test/resources/sentry-site_no_oo.xml.template
D fe/src/test/resources/sentry-site_oo.xml.template
D fe/src/test/resources/sentry-site_oo_nogrant.xml.template
11 files changed, 310 insertions(+), 997 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ief4434d80baae0fd7be7ffe7b2e07bae1ac45e47
Gerrit-Change-Number: 12930
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>

[Impala-ASF-CR] Clean up generation of XML configuration files

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

Change subject: Clean up generation of XML configuration files
......................................................................


Patch Set 1:

(19 comments)

http://gerrit.cloudera.org:8080/#/c/12930/1/bin/generate_xml_config.py
File bin/generate_xml_config.py:

http://gerrit.cloudera.org:8080/#/c/12930/1/bin/generate_xml_config.py@18
PS1, Line 18: t
flake8: E501 line too long (99 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/12930/1/bin/generate_xml_config.py@33
PS1, Line 33: def _substitute_env_vars(s):
flake8: E302 expected 2 blank lines, found 1


http://gerrit.cloudera.org:8080/#/c/12930/1/bin/generate_xml_config.py@38
PS1, Line 38: def dump_config(d, source_path, out):
flake8: E302 expected 2 blank lines, found 1


http://gerrit.cloudera.org:8080/#/c/12930/1/bin/generate_xml_config.py@46
PS1, Line 46:       
flake8: W293 blank line contains whitespace


http://gerrit.cloudera.org:8080/#/c/12930/1/bin/generate_xml_config.py@46
PS1, Line 46:       
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/12930/1/bin/generate_xml_config.py@57
PS1, Line 57: )
flake8: E501 line too long (91 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/12930/1/bin/generate_xml_config.py@66
PS1, Line 66: def main():
flake8: E302 expected 2 blank lines, found 1


http://gerrit.cloudera.org:8080/#/c/12930/1/bin/generate_xml_config.py@81
PS1, Line 81: e
flake8: E722 do not use bare except'


http://gerrit.cloudera.org:8080/#/c/12930/1/bin/generate_xml_config.py@86
PS1, Line 86: if __name__ == "__main__":
flake8: E305 expected 2 blank lines after class or function definition, found 1


http://gerrit.cloudera.org:8080/#/c/12930/1/fe/src/test/resources/hive-site.xml.py
File fe/src/test/resources/hive-site.xml.py:

http://gerrit.cloudera.org:8080/#/c/12930/1/fe/src/test/resources/hive-site.xml.py@58
PS1, Line 58: 2
flake8: E501 line too long (96 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/12930/1/fe/src/test/resources/hive-site.xml.py@66
PS1, Line 66: #
flake8: E131 continuation line unaligned for hanging indent


http://gerrit.cloudera.org:8080/#/c/12930/1/fe/src/test/resources/hive-site.xml.py@67
PS1, Line 67: e
flake8: E501 line too long (94 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/12930/1/fe/src/test/resources/hive-site.xml.py@82
PS1, Line 82: '
flake8: E131 continuation line unaligned for hanging indent


http://gerrit.cloudera.org:8080/#/c/12930/1/fe/src/test/resources/hive-site.xml.py@82
PS1, Line 82: i
flake8: E501 line too long (119 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/12930/1/fe/src/test/resources/hive-site.xml.py@92
PS1, Line 92: f
flake8: E501 line too long (108 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/12930/1/fe/src/test/resources/hive-site.xml.py@93
PS1, Line 93: .
flake8: E501 line too long (117 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/12930/1/fe/src/test/resources/hive-site.xml.py@113
PS1, Line 113: t
flake8: E501 line too long (112 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/12930/1/fe/src/test/resources/hive-site.xml.py@118
PS1, Line 118: 
flake8: W391 blank line at end of file


http://gerrit.cloudera.org:8080/#/c/12930/1/fe/src/test/resources/sentry-site.xml.py
File fe/src/test/resources/sentry-site.xml.py:

http://gerrit.cloudera.org:8080/#/c/12930/1/fe/src/test/resources/sentry-site.xml.py@49
PS1, Line 49: r
flake8: E501 line too long (95 > 90 characters)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ief4434d80baae0fd7be7ffe7b2e07bae1ac45e47
Gerrit-Change-Number: 12930
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Apr 2019 05:51:45 +0000
Gerrit-HasComments: Yes