From fde874ea568f640d768431f26e0e7a5f72f22ad3 Mon Sep 17 00:00:00 2001 From: fjy Date: Tue, 2 Jul 2013 16:11:12 -0700 Subject: [PATCH] fix according to code review --- .../curator/inventory/CuratorInventoryManager.java | 4 ---- .../druid/aggregation/CountAggregatorFactory.java | 2 +- .../druid/aggregation/DoubleSumAggregatorFactory.java | 4 ++-- .../druid/aggregation/HistogramAggregatorFactory.java | 4 ++-- .../druid/aggregation/JavaScriptAggregatorFactory.java | 10 +++++----- .../druid/aggregation/LongSumAggregatorFactory.java | 4 ++-- .../metamx/druid/aggregation/MaxAggregatorFactory.java | 4 ++-- .../metamx/druid/aggregation/MinAggregatorFactory.java | 4 ++-- .../metamx/druid/loading/HdfsDataSegmentPusher.java | 2 +- .../druid/loading/HdfsDataSegmentPusherConfig.java | 5 +---- 10 files changed, 18 insertions(+), 25 deletions(-) diff --git a/client/src/main/java/com/metamx/druid/curator/inventory/CuratorInventoryManager.java b/client/src/main/java/com/metamx/druid/curator/inventory/CuratorInventoryManager.java index bfd75488be8..ab1e31bbc49 100644 --- a/client/src/main/java/com/metamx/druid/curator/inventory/CuratorInventoryManager.java +++ b/client/src/main/java/com/metamx/druid/curator/inventory/CuratorInventoryManager.java @@ -288,10 +288,6 @@ public class CuratorInventoryManager final String inventoryKey = ZKPaths.getNodeFromPath(child.getPath()); - if (inventoryKey == null) { - return; - } - switch (event.getType()) { case CHILD_ADDED: case CHILD_UPDATED: diff --git a/common/src/main/java/com/metamx/druid/aggregation/CountAggregatorFactory.java b/common/src/main/java/com/metamx/druid/aggregation/CountAggregatorFactory.java index f4e2265745c..55900394860 100644 --- a/common/src/main/java/com/metamx/druid/aggregation/CountAggregatorFactory.java +++ b/common/src/main/java/com/metamx/druid/aggregation/CountAggregatorFactory.java @@ -41,7 +41,7 @@ public class CountAggregatorFactory implements AggregatorFactory @JsonProperty("name") String name ) { - Preconditions.checkNotNull(name, "Must have a valid, non null aggregator name"); + Preconditions.checkNotNull(name, "Must have a valid, non-null aggregator name"); this.name = name; } diff --git a/common/src/main/java/com/metamx/druid/aggregation/DoubleSumAggregatorFactory.java b/common/src/main/java/com/metamx/druid/aggregation/DoubleSumAggregatorFactory.java index 0f47561d1d7..ec89a79f39d 100644 --- a/common/src/main/java/com/metamx/druid/aggregation/DoubleSumAggregatorFactory.java +++ b/common/src/main/java/com/metamx/druid/aggregation/DoubleSumAggregatorFactory.java @@ -45,8 +45,8 @@ public class DoubleSumAggregatorFactory implements AggregatorFactory @JsonProperty("fieldName") final String fieldName ) { - Preconditions.checkNotNull(name, "Must have a valid, nonl null aggregator name"); - Preconditions.checkNotNull(fieldName, "Must have a valid, non null fieldName"); + Preconditions.checkNotNull(name, "Must have a valid, non-null aggregator name"); + Preconditions.checkNotNull(fieldName, "Must have a valid, non-null fieldName"); this.name = name; this.fieldName = fieldName; diff --git a/common/src/main/java/com/metamx/druid/aggregation/HistogramAggregatorFactory.java b/common/src/main/java/com/metamx/druid/aggregation/HistogramAggregatorFactory.java index 5bc4a40a420..6c65cd95f28 100644 --- a/common/src/main/java/com/metamx/druid/aggregation/HistogramAggregatorFactory.java +++ b/common/src/main/java/com/metamx/druid/aggregation/HistogramAggregatorFactory.java @@ -51,8 +51,8 @@ public class HistogramAggregatorFactory implements AggregatorFactory @JsonProperty("breaks") final List breaksList ) { - Preconditions.checkNotNull(name, "Must have a valid, nonl null aggregator name"); - Preconditions.checkNotNull(fieldName, "Must have a valid, non null fieldName"); + Preconditions.checkNotNull(name, "Must have a valid, non-null aggregator name"); + Preconditions.checkNotNull(fieldName, "Must have a valid, non-null fieldName"); this.name = name; this.fieldName = fieldName; diff --git a/common/src/main/java/com/metamx/druid/aggregation/JavaScriptAggregatorFactory.java b/common/src/main/java/com/metamx/druid/aggregation/JavaScriptAggregatorFactory.java index 7e4c1a66c6a..a8375f294fa 100644 --- a/common/src/main/java/com/metamx/druid/aggregation/JavaScriptAggregatorFactory.java +++ b/common/src/main/java/com/metamx/druid/aggregation/JavaScriptAggregatorFactory.java @@ -63,11 +63,11 @@ public class JavaScriptAggregatorFactory implements AggregatorFactory @JsonProperty("fnCombine") final String fnCombine ) { - Preconditions.checkNotNull(name, "Must have a valid, non null aggregator name"); - Preconditions.checkNotNull(fieldNames, "Must have a valid, non null fieldNames"); - Preconditions.checkNotNull(fnAggregate, "Must have a valid, non null fnAggregate"); - Preconditions.checkNotNull(fnReset, "Must have a valid, non null fnReset"); - Preconditions.checkNotNull(fnCombine, "Must have a valid, non null fnCombine"); + Preconditions.checkNotNull(name, "Must have a valid, non-null aggregator name"); + Preconditions.checkNotNull(fieldNames, "Must have a valid, non-null fieldNames"); + Preconditions.checkNotNull(fnAggregate, "Must have a valid, non-null fnAggregate"); + Preconditions.checkNotNull(fnReset, "Must have a valid, non-null fnReset"); + Preconditions.checkNotNull(fnCombine, "Must have a valid, non-null fnCombine"); this.name = name; this.fieldNames = fieldNames; diff --git a/common/src/main/java/com/metamx/druid/aggregation/LongSumAggregatorFactory.java b/common/src/main/java/com/metamx/druid/aggregation/LongSumAggregatorFactory.java index 1aeb5e6471d..07e04254f76 100644 --- a/common/src/main/java/com/metamx/druid/aggregation/LongSumAggregatorFactory.java +++ b/common/src/main/java/com/metamx/druid/aggregation/LongSumAggregatorFactory.java @@ -45,8 +45,8 @@ public class LongSumAggregatorFactory implements AggregatorFactory @JsonProperty("fieldName") final String fieldName ) { - Preconditions.checkNotNull(name, "Must have a valid, nonl null aggregator name"); - Preconditions.checkNotNull(fieldName, "Must have a valid, non null fieldName"); + Preconditions.checkNotNull(name, "Must have a valid, non-null aggregator name"); + Preconditions.checkNotNull(fieldName, "Must have a valid, non-null fieldName"); this.name = name; this.fieldName = fieldName; diff --git a/common/src/main/java/com/metamx/druid/aggregation/MaxAggregatorFactory.java b/common/src/main/java/com/metamx/druid/aggregation/MaxAggregatorFactory.java index dda25b355d7..45cd85257f7 100644 --- a/common/src/main/java/com/metamx/druid/aggregation/MaxAggregatorFactory.java +++ b/common/src/main/java/com/metamx/druid/aggregation/MaxAggregatorFactory.java @@ -45,8 +45,8 @@ public class MaxAggregatorFactory implements AggregatorFactory @JsonProperty("fieldName") final String fieldName ) { - Preconditions.checkNotNull(name, "Must have a valid, non null aggregator name"); - Preconditions.checkNotNull(fieldName, "Must have a valid, non null fieldName"); + Preconditions.checkNotNull(name, "Must have a valid, non-null aggregator name"); + Preconditions.checkNotNull(fieldName, "Must have a valid, non-null fieldName"); this.name = name; this.fieldName = fieldName; diff --git a/common/src/main/java/com/metamx/druid/aggregation/MinAggregatorFactory.java b/common/src/main/java/com/metamx/druid/aggregation/MinAggregatorFactory.java index 5b92072be07..a6d19ebd8e5 100644 --- a/common/src/main/java/com/metamx/druid/aggregation/MinAggregatorFactory.java +++ b/common/src/main/java/com/metamx/druid/aggregation/MinAggregatorFactory.java @@ -45,8 +45,8 @@ public class MinAggregatorFactory implements AggregatorFactory @JsonProperty("fieldName") final String fieldName ) { - Preconditions.checkNotNull(name, "Must have a valid, non null aggregator name"); - Preconditions.checkNotNull(fieldName, "Must have a valid, non null fieldName"); + Preconditions.checkNotNull(name, "Must have a valid, non-null aggregator name"); + Preconditions.checkNotNull(fieldName, "Must have a valid, non-null fieldName"); this.name = name; this.fieldName = fieldName; diff --git a/server/src/main/java/com/metamx/druid/loading/HdfsDataSegmentPusher.java b/server/src/main/java/com/metamx/druid/loading/HdfsDataSegmentPusher.java index 09fdf72f863..52ac15129d4 100644 --- a/server/src/main/java/com/metamx/druid/loading/HdfsDataSegmentPusher.java +++ b/server/src/main/java/com/metamx/druid/loading/HdfsDataSegmentPusher.java @@ -43,7 +43,7 @@ public class HdfsDataSegmentPusher implements DataSegmentPusher public DataSegment push(File inDir, DataSegment segment) throws IOException { final String storageDir = DataSegmentPusherUtil.getStorageDir(segment); - Path outFile = config.getStorageDirectory().suffix(String.format("/%s/index.zip", storageDir)); + Path outFile = new Path(String.format("%s/%s/index.zip", config.getStorageDirectory(), storageDir)); FileSystem fs = outFile.getFileSystem(hadoopConfig); fs.mkdirs(outFile.getParent()); diff --git a/server/src/main/java/com/metamx/druid/loading/HdfsDataSegmentPusherConfig.java b/server/src/main/java/com/metamx/druid/loading/HdfsDataSegmentPusherConfig.java index 70f3c979388..b27d03672bc 100644 --- a/server/src/main/java/com/metamx/druid/loading/HdfsDataSegmentPusherConfig.java +++ b/server/src/main/java/com/metamx/druid/loading/HdfsDataSegmentPusherConfig.java @@ -19,15 +19,12 @@ package com.metamx.druid.loading; -import org.apache.hadoop.fs.Path; import org.skife.config.Config; -import java.io.File; - /** */ public abstract class HdfsDataSegmentPusherConfig { @Config("druid.pusher.hdfs.storageDirectory") - public abstract Path getStorageDirectory(); + public abstract String getStorageDirectory(); }