Re: Review Request 50109: Add check for null LoggingRequestHelper in the LogSearch Integration LogSearchDataRetrievalService class
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50109/#review142500 --- Ship it! Ship It! - Oliver Szabo On July 16, 2016, 6:53 p.m., Robert Nettleton wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50109/ > --- > > (Updated July 16, 2016, 6:53 p.m.) > > > Review request for Ambari, Mahadev Konar, Oliver Szabo, and Sumit Mohanty. > > > Bugs: AMBARI-17758 > https://issues.apache.org/jira/browse/AMBARI-17758 > > > Repository: ambari > > > Description > --- > > This patch resolves AMBARI-17758. > > The "LogSearchDataRetrievalService.getLogFileTailURI()" method attempts to > obtain a LoggingRequestHelper instance in order to service a request, but a > NullPointerException can be thrown if the service is not available for some > reason. > > This patch resolves this issue by implementing the following: > > 1. Adding a binding in ControllerModule in Ambari, so that the > LoggingRequestHelperFactory is bound to LoggingRequestHelperFactoryImpl, and > can be injected into the LogSearchDataRetrievalService class. > 2. Refactors the LogSearchDataRetrievalService to use an injected instance of > the LoggingRequestHelperFactory, rather than a hard-coded call to a > constructor. > 3. Updates the LogSearchDataRetrievalService.getLogFileTailURI() method, to > check for a null LoggingRequestHelper returned from the associated factory. > This helper instance is usually null only when the LogSearch Server is not > available, or if LogSearch is not included in the current cluster. > 4. Updates the LoggingSearchPropertyProvider, which calls the > LogSearchDataRetrievalService.getLogFileTailURI() method. If a null value is > returned from this call for the tail file URI, then no log file definitions > are added to the current host component, since LogSearch is not up and > running. > 5. Adds a new unit test class for the LogSearchDataRetrievalService class, > and adds two new tests to verify the changes in this current patch. > 6. Updates the LoggingSearchPropertyProviderTest unit test, to accomodate the > changes in this patch. > > > Diffs > - > > > ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java > b4237a91 > > ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LogSearchDataRetrievalService.java > 877f4e3 > > ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LoggingSearchPropertyProvider.java > 32690e8 > > ambari-server/src/test/java/org/apache/ambari/server/controller/logging/LogSearchDataRetrievalServiceTest.java > PRE-CREATION > > ambari-server/src/test/java/org/apache/ambari/server/controller/logging/LoggingSearchPropertyProviderTest.java > 30b32d8 > > Diff: https://reviews.apache.org/r/50109/diff/ > > > Testing > --- > > 1. Deployed a 3-node cluster with HDFS, Yarn, Zookeeper, Metrics, and > LogSearch selected, with this patch applied. Verified that the cluster > deployed properly. Verified that the LogSearch integration APIs are working > as expected. Also ran the HDFS HA NameNode wizard on this cluster, and > verified that this works properly. > 2. Ran the ambari-server "mvn clean test" suite on trunk with this patch > applied, and the suite passes completely. > > > Thanks, > > Robert Nettleton > >
Re: Review Request 50109: Add check for null LoggingRequestHelper in the LogSearch Integration LogSearchDataRetrievalService class
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50109/#review142495 --- Ship it! Ship It! - Mahadev Konar On July 16, 2016, 6:53 p.m., Robert Nettleton wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50109/ > --- > > (Updated July 16, 2016, 6:53 p.m.) > > > Review request for Ambari, Mahadev Konar, Oliver Szabo, and Sumit Mohanty. > > > Bugs: AMBARI-17758 > https://issues.apache.org/jira/browse/AMBARI-17758 > > > Repository: ambari > > > Description > --- > > This patch resolves AMBARI-17758. > > The "LogSearchDataRetrievalService.getLogFileTailURI()" method attempts to > obtain a LoggingRequestHelper instance in order to service a request, but a > NullPointerException can be thrown if the service is not available for some > reason. > > This patch resolves this issue by implementing the following: > > 1. Adding a binding in ControllerModule in Ambari, so that the > LoggingRequestHelperFactory is bound to LoggingRequestHelperFactoryImpl, and > can be injected into the LogSearchDataRetrievalService class. > 2. Refactors the LogSearchDataRetrievalService to use an injected instance of > the LoggingRequestHelperFactory, rather than a hard-coded call to a > constructor. > 3. Updates the LogSearchDataRetrievalService.getLogFileTailURI() method, to > check for a null LoggingRequestHelper returned from the associated factory. > This helper instance is usually null only when the LogSearch Server is not > available, or if LogSearch is not included in the current cluster. > 4. Updates the LoggingSearchPropertyProvider, which calls the > LogSearchDataRetrievalService.getLogFileTailURI() method. If a null value is > returned from this call for the tail file URI, then no log file definitions > are added to the current host component, since LogSearch is not up and > running. > 5. Adds a new unit test class for the LogSearchDataRetrievalService class, > and adds two new tests to verify the changes in this current patch. > 6. Updates the LoggingSearchPropertyProviderTest unit test, to accomodate the > changes in this patch. > > > Diffs > - > > > ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java > b4237a91 > > ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LogSearchDataRetrievalService.java > 877f4e3 > > ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LoggingSearchPropertyProvider.java > 32690e8 > > ambari-server/src/test/java/org/apache/ambari/server/controller/logging/LogSearchDataRetrievalServiceTest.java > PRE-CREATION > > ambari-server/src/test/java/org/apache/ambari/server/controller/logging/LoggingSearchPropertyProviderTest.java > 30b32d8 > > Diff: https://reviews.apache.org/r/50109/diff/ > > > Testing > --- > > 1. Deployed a 3-node cluster with HDFS, Yarn, Zookeeper, Metrics, and > LogSearch selected, with this patch applied. Verified that the cluster > deployed properly. Verified that the LogSearch integration APIs are working > as expected. Also ran the HDFS HA NameNode wizard on this cluster, and > verified that this works properly. > 2. Ran the ambari-server "mvn clean test" suite on trunk with this patch > applied, and the suite passes completely. > > > Thanks, > > Robert Nettleton > >