----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60501/#review179121 -----------------------------------------------------------
Fix it, then Ship it! Patch looks fine to me, other than the minor issue listed below. ambari-logsearch/ambari-logsearch-config-api/src/main/java/org/apache/ambari/logsearch/config/api/LogSearchPropertyDescription.java Lines 47 (patched) <https://reviews.apache.org/r/60501/#comment253631> I would recommend making the example value be an array of list (or some other Collection) of Strings. If the property only accepts an enumerated set of options, it would be a good idea to be able to specify these in the markup. It would also be good to be able to distinguish between a property that takes a comma-separated list of values, as opposed to a property that lists the possible valid values. Check out the following class in ambari-server, which provides a similar markdown for config properties: org.apache.ambari.annotations.Markdown - Robert Nettleton On June 28, 2017, 3:51 p.m., Oliver Szabo wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60501/ > ----------------------------------------------------------- > > (Updated June 28, 2017, 3:51 p.m.) > > > Review request for Ambari, Andrii Babiichuk, Miklos Gergely, and Robert > Nettleton. > > > Bugs: AMBARI-21366 > https://issues.apache.org/jira/browse/AMBARI-21366 > > > Repository: ambari > > > Description > ------- > > - added new annotation to create descrptions for all internal logsearch > properties (currently only for logsearch.properties, not logfeeder.properties) > - create /info endpoints to get access the property descriptions > (/api/v1/info/properties) > - speed up doc generation (for this change and for swagger as well...as it > generated every time you hit the rest api call, now it will generated during > startup on a thread, from that point you will be able to acces the docs from > the memory) > > Also some UI changes: > - there was a /api/v1/pulbic/config call, instead of it, uses > /api/v1/info/auth with returning a simple map json (not that weird name value > structure)...use that endpoint in login.html > > > Diffs > ----- > > > ambari-logsearch/ambari-logsearch-config-api/src/main/java/org/apache/ambari/logsearch/config/api/LogSearchPropertyDescription.java > PRE-CREATION > > ambari-logsearch/ambari-logsearch-server/src/main/java/org/apache/ambari/logsearch/LogSearch.java > 45281b6 > > ambari-logsearch/ambari-logsearch-server/src/main/java/org/apache/ambari/logsearch/common/ApiDocStorage.java > PRE-CREATION > > ambari-logsearch/ambari-logsearch-server/src/main/java/org/apache/ambari/logsearch/common/PropertiesHelper.java > a2a7f5e > > ambari-logsearch/ambari-logsearch-server/src/main/java/org/apache/ambari/logsearch/common/PropertyDescriptionStorage.java > PRE-CREATION > > ambari-logsearch/ambari-logsearch-server/src/main/java/org/apache/ambari/logsearch/conf/ApiDocConfig.java > 9d4e96e > > ambari-logsearch/ambari-logsearch-server/src/main/java/org/apache/ambari/logsearch/conf/AuthPropsConfig.java > 54cc10c > > ambari-logsearch/ambari-logsearch-server/src/main/java/org/apache/ambari/logsearch/conf/LogSearchHttpHeaderConfig.java > cb8c097 > > ambari-logsearch/ambari-logsearch-server/src/main/java/org/apache/ambari/logsearch/conf/SecurityConfig.java > 441b628 > > ambari-logsearch/ambari-logsearch-server/src/main/java/org/apache/ambari/logsearch/conf/SolrAuditLogPropsConfig.java > 5981bcc > > ambari-logsearch/ambari-logsearch-server/src/main/java/org/apache/ambari/logsearch/conf/SolrConnectionPropsConfig.java > 7d37efd > > ambari-logsearch/ambari-logsearch-server/src/main/java/org/apache/ambari/logsearch/conf/SolrEventHistoryPropsConfig.java > a117501 > > ambari-logsearch/ambari-logsearch-server/src/main/java/org/apache/ambari/logsearch/conf/SolrKerberosConfig.java > 7cf79b0 > > ambari-logsearch/ambari-logsearch-server/src/main/java/org/apache/ambari/logsearch/conf/SolrServiceLogPropsConfig.java > 482438a > > ambari-logsearch/ambari-logsearch-server/src/main/java/org/apache/ambari/logsearch/doc/DocConstants.java > 73de0ee > > ambari-logsearch/ambari-logsearch-server/src/main/java/org/apache/ambari/logsearch/manager/InfoManager.java > PRE-CREATION > > ambari-logsearch/ambari-logsearch-server/src/main/java/org/apache/ambari/logsearch/manager/PublicManager.java > cbab651 > > ambari-logsearch/ambari-logsearch-server/src/main/java/org/apache/ambari/logsearch/model/response/PropertyDescriptionData.java > PRE-CREATION > > ambari-logsearch/ambari-logsearch-server/src/main/java/org/apache/ambari/logsearch/rest/InfoResource.java > PRE-CREATION > > ambari-logsearch/ambari-logsearch-server/src/main/java/org/apache/ambari/logsearch/rest/PublicResource.java > df83d44 > > ambari-logsearch/ambari-logsearch-server/src/main/java/org/apache/ambari/logsearch/rest/SwaggerResource.java > PRE-CREATION > > ambari-logsearch/ambari-logsearch-server/src/main/java/org/apache/ambari/logsearch/util/SSLUtil.java > d4b6544 > > ambari-logsearch/ambari-logsearch-server/src/main/java/org/apache/ambari/logsearch/web/filters/LogsearchKRBAuthenticationFilter.java > 1b77753 > > ambari-logsearch/ambari-logsearch-server/src/main/java/org/apache/ambari/logsearch/web/security/LogsearchExternalServerAuthenticationProvider.java > 1dab126 > > ambari-logsearch/ambari-logsearch-server/src/test/java/org/apache/ambari/logsearch/web/security/LogsearchExternalServerAuthenticationProviderTest.java > d6247a1 > ambari-logsearch/ambari-logsearch-web/src/main/webapp/login.html f2cb2c7 > > > Diff: https://reviews.apache.org/r/60501/diff/3/ > > > Testing > ------- > > done. manually & check /docs endpoint > > > Thanks, > > Oliver Szabo > >
