-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63477/#review189837
-----------------------------------------------------------


Fix it, then Ship it!





ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/resources/config.yml
Lines 27 (patched)
<https://reviews.apache.org/r/63477/#comment267065>

    /etc/ams-adservice, similar to others?



ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/app/AnomalyDetectionAppConfig.scala
Lines 25 (patched)
<https://reviews.apache.org/r/63477/#comment267066>

    Make sure imports are properly alligned based on project rules.



ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/app/AnomalyDetectionAppConfig.scala
Lines 39 (patched)
<https://reviews.apache.org/r/63477/#comment267067>

    Why var?



ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/configuration/HBaseConfiguration.scala
Lines 26 (patched)
<https://reviews.apache.org/r/63477/#comment267068>

    Why var?



ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/configuration/MetricManagerServiceConfiguration.scala
Lines 36 (patched)
<https://reviews.apache.org/r/63477/#comment267069>

    Is setter needed?



ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/db/AdMetadataStoreAccessor.scala
Lines 30 (patched)
<https://reviews.apache.org/r/63477/#comment267070>

    This behavior should be left upto the impl and not a part of trait behavior



ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/db/PhoenixAnomalyStoreAccessor.scala
Lines 81 (patched)
<https://reviews.apache.org/r/63477/#comment267071>

    Convoluted naming convention.



ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/metricmanager/AMSMetadataProvider.scala
Lines 32 (patched)
<https://reviews.apache.org/r/63477/#comment267072>

    Rename to ADMetadataProvider



ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/metricmanager/AMSMetadataProvider.scala
Lines 34 (patched)
<https://reviews.apache.org/r/63477/#comment267073>

    Prefer immutables here.



ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/metricmanager/AMSMetadataProvider.scala
Lines 54 (patched)
<https://reviews.apache.org/r/63477/#comment267074>

    Unused?



ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/metricmanager/MetricManagerServiceImpl.scala
Lines 29 (patched)
<https://reviews.apache.org/r/63477/#comment267077>

    Immutable should be preferred !



ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/metricmanager/MetricManagerServiceImpl.scala
Lines 62 (patched)
<https://reviews.apache.org/r/63477/#comment267078>

    Service initialization should not make a blocking http call rather fail 
late when API call is made. Log error when failing.



ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/HBaseTimelineMetricsService.java
Lines 469 (patched)
<https://reviews.apache.org/r/63477/#comment267079>

    formatting.


- Sid Wagle


On Nov. 1, 2017, 9:21 p.m., Aravindan Vijayan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63477/
> -----------------------------------------------------------
> 
> (Updated Nov. 1, 2017, 9:21 p.m.)
> 
> 
> Review request for Ambari, Jayush Luniya and Sid Wagle.
> 
> 
> Bugs: AMBARI-22348
>     https://issues.apache.org/jira/browse/AMBARI-22348
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Initial implementation for
> . Confuguration Handling for the AD Service
> . Model classes for Anomalies, Metric Definition, Metric Key etc.
> . Seasonality Handling
> . Metric Definition Service
> . Unit testing
> 
> 
> Diffs
> -----
> 
>   ambari-metrics/ambari-metrics-anomaly-detection-service/pom.xml 554d026 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/resources/config.yml
>  9ca9e95 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/app/AnomalyDetectionAppConfig.scala
>  9e6cc6d 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/common/ADServiceConfiguration.scala
>  248c74e 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/common/PhoenixQueryConstants.scala
>  17173ec 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/common/Range.scala
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/common/Season.scala
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/common/SeasonType.scala
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/common/SeasonalityHandler.scala
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/common/TimeRange.scala
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/configuration/AdServiceConfiguration.scala
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/configuration/HBaseConfiguration.scala
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/configuration/MetricCollectorConfiguration.scala
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/configuration/MetricManagerServiceConfiguration.scala
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/db/AdMetadataStoreAccessor.scala
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/db/AdMetadataStoreConstants.scala
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/db/PhoenixAnomalyStoreAccessor.scala
>  6f33e56 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/db/PhoenixQueryConstants.scala
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/metricmanager/AMSMetadataProvider.scala
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/metricmanager/InputMetricDefinitionParser.scala
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/metricmanager/MetricDefinition.scala
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/metricmanager/MetricKey.scala
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/metricmanager/MetricManagerService.scala
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/metricmanager/MetricManagerServiceImpl.scala
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/metricmanager/MetricMetadataProvider.scala
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/metricmanager/MetricSourceDefinition.scala
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/metricmanager/MetricSourceDefinitionType.scala
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/model/AnomalyDetectionMethod.scala
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/model/AnomalyType.scala
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/model/SingleMetricAnomalyInstance.scala
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/resource/MetricDefinitionResource.scala
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/resource/SubsystemResource.scala
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/service/ADQueryService.scala
>  0161166 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/service/ADQueryServiceImpl.scala
>  fe00f58 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/subsystem/pointintime/PointInTimeAnomalyInstance.scala
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/main/scala/org/apache/ambari/metrics/adservice/subsystem/trend/TrendAnomalyInstance.scala
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/test/scala/org/apache/ambari/metrics/adservice/app/AnomalyDetectionAppConfigTest.scala
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/test/scala/org/apache/ambari/metrics/adservice/common/ADManagerConfigurationTest.scala
>  40b9d6a 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/test/scala/org/apache/ambari/metrics/adservice/common/RangeTest.scala
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/test/scala/org/apache/ambari/metrics/adservice/common/SeasonTest.scala
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/test/scala/org/apache/ambari/metrics/adservice/common/SeasonalityHandlerTest.scala
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/test/scala/org/apache/ambari/metrics/adservice/metricmanager/AMSMetadataProviderTest.scala
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/test/scala/org/apache/ambari/metrics/adservice/metricmanager/MetricManagerServiceTest.scala
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-anomaly-detection-service/src/test/scala/org/apache/ambari/metrics/adservice/metricmanager/MetricSourceDefinitionTest.scala
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-common/src/main/java/org/apache/hadoop/metrics2/sink/timeline/TimelineMetricKey.java
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/HBaseTimelineMetricsService.java
>  bb26439 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/TimelineMetricStore.java
>  cdeefdc 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/discovery/TimelineMetricMetadataManager.java
>  f9ad773 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/webapp/TimelineWebServices.java
>  5d9bb35 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/TestTimelineMetricStore.java
>  12bd463 
> 
> 
> Diff: https://reviews.apache.org/r/63477/diff/2/
> 
> 
> Testing
> -------
> 
> Unit Testing.
> 
> 
> Thanks,
> 
> Aravindan Vijayan
> 
>

Reply via email to