You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@curator.apache.org by ca...@apache.org on 2014/08/04 23:33:34 UTC

[1/4] git commit: CURATOR-67 defer creation of serializer

Repository: curator
Updated Branches:
  refs/heads/CURATOR-67 [created] fd8b64768


CURATOR-67 defer creation of serializer

Instead of immediately creating a new JSONInstanceSerializer we can
defer until build() is actually called. This lets users specify their
own serializers and avoids issues where there may be version conflicts.


Project: http://git-wip-us.apache.org/repos/asf/curator/repo
Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/f24e11bd
Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/f24e11bd
Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/f24e11bd

Branch: refs/heads/CURATOR-67
Commit: f24e11bdc9ca45e94479b1c6fc981b8b7393f619
Parents: f5767c8
Author: Mike Drob <md...@cloudera.com>
Authored: Fri Aug 1 11:29:30 2014 -0500
Committer: Mike Drob <md...@cloudera.com>
Committed: Fri Aug 1 11:29:30 2014 -0500

----------------------------------------------------------------------
 .../x/discovery/ServiceDiscoveryBuilder.java    |  7 +-
 .../details/TestServiceDiscoveryBuilder.java    | 68 ++++++++++++++++++++
 2 files changed, 74 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/curator/blob/f24e11bd/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/ServiceDiscoveryBuilder.java
----------------------------------------------------------------------
diff --git a/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/ServiceDiscoveryBuilder.java b/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/ServiceDiscoveryBuilder.java
index ab62004..28fc31c 100644
--- a/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/ServiceDiscoveryBuilder.java
+++ b/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/ServiceDiscoveryBuilder.java
@@ -29,6 +29,7 @@ public class ServiceDiscoveryBuilder<T>
     private String                  basePath;
     private InstanceSerializer<T>   serializer;
     private ServiceInstance<T>      thisInstance;
+    private Class<T>                payloadClass;
 
     /**
      * Return a new builder. The builder will be defaulted with a {@link JsonInstanceSerializer}.
@@ -39,7 +40,7 @@ public class ServiceDiscoveryBuilder<T>
      */
     public static<T> ServiceDiscoveryBuilder<T>     builder(Class<T> payloadClass)
     {
-        return new ServiceDiscoveryBuilder<T>(payloadClass).serializer(new JsonInstanceSerializer<T>(payloadClass));
+        return new ServiceDiscoveryBuilder<T>(payloadClass);
     }
 
     /**
@@ -49,6 +50,9 @@ public class ServiceDiscoveryBuilder<T>
      */
     public ServiceDiscovery<T>      build()
     {
+        if ( serializer == null ) {
+            serializer(new JsonInstanceSerializer<T>(payloadClass));
+        }
         return new ServiceDiscoveryImpl<T>(client, basePath, serializer, thisInstance);
     }
 
@@ -102,5 +106,6 @@ public class ServiceDiscoveryBuilder<T>
 
     ServiceDiscoveryBuilder(Class<T> payloadClass)
     {
+        this.payloadClass = payloadClass;
     }
 }

http://git-wip-us.apache.org/repos/asf/curator/blob/f24e11bd/curator-x-discovery/src/test/java/org/apache/curator/x/discovery/details/TestServiceDiscoveryBuilder.java
----------------------------------------------------------------------
diff --git a/curator-x-discovery/src/test/java/org/apache/curator/x/discovery/details/TestServiceDiscoveryBuilder.java b/curator-x-discovery/src/test/java/org/apache/curator/x/discovery/details/TestServiceDiscoveryBuilder.java
new file mode 100644
index 0000000..072eabf
--- /dev/null
+++ b/curator-x-discovery/src/test/java/org/apache/curator/x/discovery/details/TestServiceDiscoveryBuilder.java
@@ -0,0 +1,68 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.curator.x.discovery.details;
+
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.curator.framework.CuratorFrameworkFactory;
+import org.apache.curator.retry.RetryOneTime;
+import org.apache.curator.test.BaseClassForTests;
+import org.apache.curator.x.discovery.ServiceDiscoveryBuilder;
+import org.apache.curator.x.discovery.ServiceInstance;
+import org.testng.Assert;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+public class TestServiceDiscoveryBuilder extends BaseClassForTests
+{
+    @Test
+    public void testDefaultSerializer() throws Exception
+    {        
+        CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(), new RetryOneTime(1));
+        ServiceDiscoveryBuilder<Object> builder = ServiceDiscoveryBuilder.builder(Object.class).client(client);
+        ServiceDiscoveryImpl<?> discovery = (ServiceDiscoveryImpl<?>) builder.basePath("/path").build();
+
+        Assert.assertNotNull(discovery.getSerializer(), "default serializer not set");
+        Assert.assertTrue(discovery.getSerializer() instanceof JsonInstanceSerializer, "default serializer not JSON");
+    }
+
+    @Test
+    public void testSetSerializer() throws Exception
+    {
+        CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(), new RetryOneTime(1));
+        ServiceDiscoveryBuilder<Object> builder = ServiceDiscoveryBuilder.builder(Object.class).client(client);
+        builder.serializer(new InstanceSerializer<Object>()
+        {
+            @Override
+            public byte[] serialize(ServiceInstance<Object> instance)
+            {
+                return null;
+            }
+
+            @Override
+            public ServiceInstance<Object> deserialize(byte[] bytes)
+            {
+                return null;
+            }
+        });
+
+        ServiceDiscoveryImpl<?> discovery = (ServiceDiscoveryImpl<?>) builder.basePath("/path").build();
+        Assert.assertNotNull(discovery.getSerializer(), "default serializer not set");
+        Assert.assertFalse(discovery.getSerializer() instanceof JsonInstanceSerializer, "set serializer is JSON");
+    }
+}


[4/4] git commit: CURATOR-67 - Fixed up comment on builder() method.

Posted by ca...@apache.org.
CURATOR-67 - Fixed up comment on builder() method.

Project: http://git-wip-us.apache.org/repos/asf/curator/repo
Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/fd8b6476
Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/fd8b6476
Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/fd8b6476

Branch: refs/heads/CURATOR-67
Commit: fd8b647680496519fbb6f1e06075dbc6d385a762
Parents: 91c9ae3
Author: Cam McKenzie <ca...@apache.org>
Authored: Tue Aug 5 07:33:12 2014 +1000
Committer: Cam McKenzie <ca...@apache.org>
Committed: Tue Aug 5 07:33:12 2014 +1000

----------------------------------------------------------------------
 .../org/apache/curator/x/discovery/ServiceDiscoveryBuilder.java    | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/curator/blob/fd8b6476/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/ServiceDiscoveryBuilder.java
----------------------------------------------------------------------
diff --git a/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/ServiceDiscoveryBuilder.java b/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/ServiceDiscoveryBuilder.java
index 28fc31c..6595d6a 100644
--- a/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/ServiceDiscoveryBuilder.java
+++ b/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/ServiceDiscoveryBuilder.java
@@ -32,7 +32,7 @@ public class ServiceDiscoveryBuilder<T>
     private Class<T>                payloadClass;
 
     /**
-     * Return a new builder. The builder will be defaulted with a {@link JsonInstanceSerializer}.
+     * Return a new builder.
      *
      * @param payloadClass the class of the payload of your service instance (you can use {@link Void}
      * if your instances don't need a payload)


[3/4] git commit: Merge branch 'CURATOR-67' of https://github.com/madrob/curator into CURATOR-67

Posted by ca...@apache.org.
Merge branch 'CURATOR-67' of https://github.com/madrob/curator into CURATOR-67


Project: http://git-wip-us.apache.org/repos/asf/curator/repo
Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/91c9ae32
Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/91c9ae32
Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/91c9ae32

Branch: refs/heads/CURATOR-67
Commit: 91c9ae32e6df0cca2ff8e0b758f27e60f8768520
Parents: 35278cc f24e11b
Author: Cam McKenzie <ca...@apache.org>
Authored: Mon Aug 4 10:42:07 2014 +1000
Committer: Cam McKenzie <ca...@apache.org>
Committed: Mon Aug 4 10:42:07 2014 +1000

----------------------------------------------------------------------
 .../x/discovery/ServiceDiscoveryBuilder.java    |  7 +-
 .../details/TestServiceDiscoveryBuilder.java    | 68 ++++++++++++++++++++
 2 files changed, 74 insertions(+), 1 deletion(-)
----------------------------------------------------------------------



[2/4] git commit: CURATOR-71 do not log-and-throw

Posted by ca...@apache.org.
CURATOR-71 do not log-and-throw

If we throw an exception, then there is no point in logging it since
something further up the call stack already has to deal with it. At
best, the exception gets logged twice, at worst we log potentially
confusing exceptions that end up not mattering.


Project: http://git-wip-us.apache.org/repos/asf/curator/repo
Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/35278cc1
Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/35278cc1
Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/35278cc1

Branch: refs/heads/CURATOR-67
Commit: 35278cc1b52802995ee5db017bcc84dba7c69a70
Parents: f5767c8
Author: Mike Drob <md...@cloudera.com>
Authored: Fri Aug 1 12:08:47 2014 -0500
Committer: Mike Drob <md...@cloudera.com>
Committed: Fri Aug 1 12:08:47 2014 -0500

----------------------------------------------------------------------
 .../main/java/org/apache/curator/CuratorZookeeperClient.java    | 5 ++---
 .../org/apache/curator/framework/imps/CuratorFrameworkImpl.java | 5 ++---
 .../apache/curator/framework/recipes/leader/LeaderSelector.java | 1 -
 .../framework/recipes/locks/StandardLockInternalsDriver.java    | 1 -
 .../framework/recipes/nodes/PersistentEphemeralNode.java        | 5 +----
 5 files changed, 5 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/curator/blob/35278cc1/curator-client/src/main/java/org/apache/curator/CuratorZookeeperClient.java
----------------------------------------------------------------------
diff --git a/curator-client/src/main/java/org/apache/curator/CuratorZookeeperClient.java b/curator-client/src/main/java/org/apache/curator/CuratorZookeeperClient.java
index f14214f..09b28b2 100644
--- a/curator-client/src/main/java/org/apache/curator/CuratorZookeeperClient.java
+++ b/curator-client/src/main/java/org/apache/curator/CuratorZookeeperClient.java
@@ -182,9 +182,8 @@ public class CuratorZookeeperClient implements Closeable
 
         if ( !started.compareAndSet(false, true) )
         {
-            IllegalStateException error = new IllegalStateException();
-            log.error("Already started", error);
-            throw error;
+            IllegalStateException ise = new IllegalStateException("Already started");
+            throw ise;
         }
 
         state.start();

http://git-wip-us.apache.org/repos/asf/curator/blob/35278cc1/curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
----------------------------------------------------------------------
diff --git a/curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java b/curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
index 01cacee..e86b1e5 100644
--- a/curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
+++ b/curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
@@ -233,9 +233,8 @@ public class CuratorFrameworkImpl implements CuratorFramework
         log.info("Starting");
         if ( !state.compareAndSet(CuratorFrameworkState.LATENT, CuratorFrameworkState.STARTED) )
         {
-            IllegalStateException error = new IllegalStateException();
-            log.error("Cannot be started more than once", error);
-            throw error;
+            IllegalStateException ise = new IllegalStateException("Cannot be started more than once");
+            throw ise;
         }
 
         try

http://git-wip-us.apache.org/repos/asf/curator/blob/35278cc1/curator-recipes/src/main/java/org/apache/curator/framework/recipes/leader/LeaderSelector.java
----------------------------------------------------------------------
diff --git a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/leader/LeaderSelector.java b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/leader/LeaderSelector.java
index e7b61f9..3be941a 100644
--- a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/leader/LeaderSelector.java
+++ b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/leader/LeaderSelector.java
@@ -418,7 +418,6 @@ public class LeaderSelector implements Closeable
         }
         catch ( Exception e )
         {
-            log.error("mutex.acquire() threw an exception", e);
             throw e;
         }
         finally

http://git-wip-us.apache.org/repos/asf/curator/blob/35278cc1/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/StandardLockInternalsDriver.java
----------------------------------------------------------------------
diff --git a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/StandardLockInternalsDriver.java b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/StandardLockInternalsDriver.java
index d225bbb..25f07b8 100644
--- a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/StandardLockInternalsDriver.java
+++ b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/StandardLockInternalsDriver.java
@@ -61,7 +61,6 @@ public class StandardLockInternalsDriver implements LockInternalsDriver
     {
         if ( ourIndex < 0 )
         {
-            log.error("Sequential path not found: " + sequenceNodeName);
             throw new KeeperException.NoNodeException("Sequential path not found: " + sequenceNodeName);
         }
     }

http://git-wip-us.apache.org/repos/asf/curator/blob/35278cc1/curator-recipes/src/main/java/org/apache/curator/framework/recipes/nodes/PersistentEphemeralNode.java
----------------------------------------------------------------------
diff --git a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/nodes/PersistentEphemeralNode.java b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/nodes/PersistentEphemeralNode.java
index 08cbe17..2b07586 100644
--- a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/nodes/PersistentEphemeralNode.java
+++ b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/nodes/PersistentEphemeralNode.java
@@ -320,7 +320,6 @@ public class PersistentEphemeralNode implements Closeable
             }
             catch ( Exception e )
             {
-                log.error("Deleting node: " + localNodePath, e);
                 throw e;
             }
         }
@@ -341,8 +340,7 @@ public class PersistentEphemeralNode implements Closeable
         }
         catch ( Exception e )
         {
-            log.error("Creating node. BasePath: " + basePath, e);
-            throw new RuntimeException(e);  // should never happen unless there's a programming error - so throw RuntimeException
+            throw new RuntimeException("Creating node. BasePath: " + basePath, e);  // should never happen unless there's a programming error - so throw RuntimeException
         }
     }
 
@@ -362,7 +360,6 @@ public class PersistentEphemeralNode implements Closeable
             }
             catch ( Exception e )
             {
-                log.error("Watching node: " + localNodePath, e);
                 throw e;
             }
         }