----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34764/#review85546 -----------------------------------------------------------
Ship it! contrib/views/hive/src/main/java/org/apache/ambari/view/hive/PropertyValidator.java <https://reviews.apache.org/r/34764/#comment137142> This method needs some JavaDocs. For example, why is `null` a valid URL? `null` seems like an invalid URL to me. contrib/views/hive/src/main/java/org/apache/ambari/view/hive/PropertyValidator.java <https://reviews.apache.org/r/34764/#comment137141> Does this meet the Ambari coding coventions? Technically it is fine, but maybe it should be converted to ``` if (webhdfsUrl == null) { return true; } ``` Note: I did notice this a few times this patch, but those instances were pre-exising code so I didn't flag them. If there are other issues to fix AND if this notation is not widely used in views code, I suggest to convert it (and the other instances) to Ambari standards. contrib/views/hive/src/main/java/org/apache/ambari/view/hive/resources/jobs/atsJobs/ATSRequestsDelegateImpl.java <https://reviews.apache.org/r/34764/#comment137143> It seems odd to have to quote (") a value in querystring. Are we sure this is correct? If so, maybe add a comment to explain. contrib/views/hive/src/main/java/org/apache/ambari/view/hive/resources/savedQueries/SavedQueryResourceManager.java <https://reviews.apache.org/r/34764/#comment137145> This method really needs a JavaDoc comment. contrib/views/hive/src/main/java/org/apache/ambari/view/hive/resources/savedQueries/SavedQueryResourceManager.java <https://reviews.apache.org/r/34764/#comment137144> `(\s+|\s?)` translates to `1 or more whitespace characters OR 0 or 1 whitspace characters`. Doesn't this really mean `0 or more whitespace characters` and thus `\s*` should be used instead? - Robert Levas On May 28, 2015, 8:02 a.m., Erik Bergenholtz wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34764/ > ----------------------------------------------------------- > > (Updated May 28, 2015, 8:02 a.m.) > > > Review request for Ambari, Robert Levas and Tom Beerbower. > > > Bugs: AMBARI-11474 > https://issues.apache.org/jira/browse/AMBARI-11474 > > > Repository: ambari > > > Description > ------- > > Just like Files View, Hive View should support both cluster association and > NameNode HA > > > Diffs > ----- > > contrib/views/hive/pom.xml da7e1a1 > > contrib/views/hive/src/main/java/org/apache/ambari/view/hive/PropertyValidator.java > 61efa49 > > contrib/views/hive/src/main/java/org/apache/ambari/view/hive/client/ConnectionFactory.java > 98256eb > > contrib/views/hive/src/main/java/org/apache/ambari/view/hive/resources/files/FileService.java > 860b2c6 > > contrib/views/hive/src/main/java/org/apache/ambari/view/hive/resources/jobs/JobService.java > ae0e828 > > contrib/views/hive/src/main/java/org/apache/ambari/view/hive/resources/jobs/atsJobs/ATSParser.java > 6e46fee > > contrib/views/hive/src/main/java/org/apache/ambari/view/hive/resources/jobs/atsJobs/ATSRequestsDelegateImpl.java > bd477a7 > > contrib/views/hive/src/main/java/org/apache/ambari/view/hive/resources/jobs/rm/RMParser.java > e1d4c73 > > contrib/views/hive/src/main/java/org/apache/ambari/view/hive/resources/jobs/rm/RMParserFactory.java > 9733147 > > contrib/views/hive/src/main/java/org/apache/ambari/view/hive/resources/jobs/rm/RMRequestsDelegateImpl.java > 43186f4 > > contrib/views/hive/src/main/java/org/apache/ambari/view/hive/resources/jobs/viewJobs/JobControllerImpl.java > eb2e141 > > contrib/views/hive/src/main/java/org/apache/ambari/view/hive/resources/savedQueries/SavedQueryResourceManager.java > 01f9c9c > > contrib/views/hive/src/main/java/org/apache/ambari/view/hive/resources/savedQueries/SavedQueryService.java > f55c1fd > > contrib/views/hive/src/main/java/org/apache/ambari/view/hive/utils/FilePaginator.java > 6282fc9 > > contrib/views/hive/src/main/java/org/apache/ambari/view/hive/utils/HdfsApi.java > cbc4d4b > > contrib/views/hive/src/main/java/org/apache/ambari/view/hive/utils/HdfsUtil.java > aeeb1b7 > > contrib/views/hive/src/main/java/org/apache/ambari/view/hive/utils/ServiceFormattedException.java > e9698c3 > > contrib/views/hive/src/main/java/org/apache/ambari/view/hive/utils/SharedObjectsFactory.java > 05ff7b6 > > contrib/views/hive/src/main/resources/ui/hive-web/app/components/progress-widget.js > d7c3fda > contrib/views/hive/src/main/resources/ui/hive-web/app/controllers/index.js > 0e4ac32 > > contrib/views/hive/src/main/resources/ui/hive-web/app/controllers/index/history-query/logs.js > c75fffe > > contrib/views/hive/src/main/resources/ui/hive-web/app/controllers/job-progress.js > 737506e > contrib/views/hive/src/main/resources/ui/hive-web/app/styles/app.scss > 6264b9e > contrib/views/hive/src/main/resources/ui/hive-web/app/utils/constants.js > 70cd374 > > contrib/views/hive/src/main/resources/ui/hive-web/app/views/visual-explain.js > 1585a40 > contrib/views/hive/src/main/resources/ui/hive-web/config/environment.js > 86da93e > contrib/views/hive/src/main/resources/view.xml 56967d2 > > contrib/views/hive/src/test/java/org/apache/ambari/view/hive/BaseHiveTest.java > f38d0e9 > > contrib/views/hive/src/test/java/org/apache/ambari/view/hive/resources/files/FileServiceTest.java > a5a0f48 > > contrib/views/hive/src/test/java/org/apache/ambari/view/hive/resources/jobs/JobServiceTest.java > 3c4202a > > contrib/views/hive/src/test/java/org/apache/ambari/view/hive/resources/savedQueries/SavedQueryResourceManagerTest.java > PRE-CREATION > > contrib/views/hive/src/test/java/org/apache/ambari/view/hive/resources/savedQueries/SavedQueryServiceTest.java > 9b26a5b > > contrib/views/hive/src/test/java/org/apache/ambari/view/hive/utils/HdfsApiMock.java > bb922e2 > > Diff: https://reviews.apache.org/r/34764/diff/ > > > Testing > ------- > > Local Unit Testing and Deployment of View. > > > Thanks, > > Erik Bergenholtz > >
