You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Vihang Karajgaonkar (Code Review)" <ge...@cloudera.org> on 2019/08/06 19:16:26 UTC

[Impala-ASF-CR] IMPALA-8761 Configuration validation introduced in IMPALA-8559 can be improved

Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/13952 )

Change subject: IMPALA-8761 Configuration validation introduced in IMPALA-8559 can be improved
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/13952/1/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java
File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java:

http://gerrit.cloudera.org:8080/#/c/13952/1/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@281
PS1, Line 281: results.size(
Instead of printing the ArrayList directly, can you add a log at error level here directly LOG.error("Found {} metastore configuration(s) incorrectly, results.size()) and then the loop below just do a LOG.error(invalidConfig.getReason());

If you do that way, you can also get rid of List<String> errorMessages.


http://gerrit.cloudera.org:8080/#/c/13952/1/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@286
PS1, Line 286:       throw new CatalogException(String.format("Found %d metastore configuration(s) incorrectly"+"" +
> line too long (101 > 90)
Please fix this line width issue. Use the IDE to reformat the changed code block.


http://gerrit.cloudera.org:8080/#/c/13952/1/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@287
PS1, Line 287:               " set. Event processing cannot be started. See error log for more details.", results.size()));
> line too long (108 > 90)
Please fix this line width issue. Use the IDE to reformat the changed code block.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52d07bce88f9332a34bfe2f9b31570203485d544
Gerrit-Change-Number: 13952
Gerrit-PatchSet: 1
Gerrit-Owner: Sharanitha Harish <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Aug 2019 19:16:26 +0000
Gerrit-HasComments: Yes