aurora-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From wfar...@apache.org
Subject [1/2] Enable some additional PMD rules, and fix sources to satisfy them.
Date Tue, 27 May 2014 18:25:09 GMT
Repository: incubator-aurora
Updated Branches:
  refs/heads/master 10da38a3a -> a8fa267f0


http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java b/src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java
index a56f037..48b36c2 100644
--- a/src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java
+++ b/src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java
@@ -58,19 +58,21 @@ public final class AsyncUtil {
         // See java.util.concurrent.ThreadPoolExecutor#afterExecute(Runnable, Throwable)
         // for more details and an implementation example.
         super.afterExecute(runnable, throwable);
-        if (throwable != null) {
-          logger.log(Level.SEVERE, throwable.toString(), throwable);
-        } else if (runnable instanceof Future) {
-          try {
-            Future<?> future = (Future<?>) runnable;
-            if (future.isDone()) {
-              future.get();
+        if (throwable == null) {
+          if (runnable instanceof Future) {
+            try {
+              Future<?> future = (Future<?>) runnable;
+              if (future.isDone()) {
+                future.get();
+              }
+            } catch (InterruptedException ie) {
+              Thread.currentThread().interrupt();
+            } catch (ExecutionException ee) {
+              logger.log(Level.SEVERE, ee.toString(), ee);
             }
-          } catch (InterruptedException ie) {
-            Thread.currentThread().interrupt();
-          } catch (ExecutionException ee) {
-            logger.log(Level.SEVERE, ee.toString(), ee);
           }
+        } else {
+          logger.log(Level.SEVERE, throwable.toString(), throwable);
         }
       }
     };

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java b/src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java
index da06f12..d885b22 100644
--- a/src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java
+++ b/src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java
@@ -28,7 +28,7 @@ public final class CommandUtil {
   }
 
   private static String uriBasename(String uri) {
-    int lastSlash = uri.lastIndexOf("/");
+    int lastSlash = uri.lastIndexOf('/');
     if (lastSlash == -1) {
       return uri;
     } else {

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/base/Query.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/base/Query.java b/src/main/java/org/apache/aurora/scheduler/base/Query.java
index a5350c8..cfb1d16 100644
--- a/src/main/java/org/apache/aurora/scheduler/base/Query.java
+++ b/src/main/java/org/apache/aurora/scheduler/base/Query.java
@@ -148,11 +148,11 @@ public final class Query {
   public static final class Builder implements Supplier<TaskQuery> {
     private final TaskQuery query;
 
-    private Builder() {
+    Builder() {
       this.query = new TaskQuery();
     }
 
-    private Builder(final TaskQuery query) {
+    Builder(final TaskQuery query) {
       this.query = checkNotNull(query); // It is expected that the caller calls deepCopy.
     }
 

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java b/src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
index f86a5b7..47cb70b 100644
--- a/src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
+++ b/src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
@@ -324,7 +324,7 @@ public final class ConfigurationManager {
 
       IValueConstraint valueConstraint = constraint.getConstraint().getValue();
 
-      if (!(valueConstraint.getValues().size() == 1)) {
+      if (valueConstraint.getValues().size() != 1) {
         throw new TaskDescriptionException("A dedicated constraint must have exactly one value");
       }
 

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/configuration/Resources.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/configuration/Resources.java b/src/main/java/org/apache/aurora/scheduler/configuration/Resources.java
index 5a38479..9ee2a85 100644
--- a/src/main/java/org/apache/aurora/scheduler/configuration/Resources.java
+++ b/src/main/java/org/apache/aurora/scheduler/configuration/Resources.java
@@ -112,7 +112,7 @@ public class Resources {
           .add(Resources.makeMesosResource(CPUS, numCpus))
           .add(Resources.makeMesosResource(DISK_MB, disk.as(Data.MB)))
           .add(Resources.makeMesosResource(RAM_MB, ram.as(Data.MB)));
-    if (selectedPorts.size() > 0) {
+    if (selectedPorts.isEmpty()) {
         resourceBuilder.add(Resources.makeMesosRangeResource(Resources.PORTS, selectedPorts));
     }
 

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/configuration/SanitizedConfiguration.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/configuration/SanitizedConfiguration.java b/src/main/java/org/apache/aurora/scheduler/configuration/SanitizedConfiguration.java
index 7fc4930..d511ec0 100644
--- a/src/main/java/org/apache/aurora/scheduler/configuration/SanitizedConfiguration.java
+++ b/src/main/java/org/apache/aurora/scheduler/configuration/SanitizedConfiguration.java
@@ -14,6 +14,7 @@
 package org.apache.aurora.scheduler.configuration;
 
 import java.util.Map;
+import java.util.logging.Logger;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Functions;
@@ -26,12 +27,15 @@ import com.google.common.collect.Range;
 import org.apache.aurora.scheduler.configuration.ConfigurationManager.TaskDescriptionException;
 import org.apache.aurora.scheduler.storage.entities.IJobConfiguration;
 import org.apache.aurora.scheduler.storage.entities.ITaskConfig;
+import org.apache.commons.lang.StringUtils;
 
 /**
  * Wrapper for a configuration that has been fully-sanitized and populated with defaults.
  */
 public final class SanitizedConfiguration {
 
+  private static final Logger LOG = Logger.getLogger(SanitizedConfiguration.class.getName());
+
   private final IJobConfiguration sanitized;
   private final Map<Integer, ITaskConfig> tasks;
 
@@ -73,6 +77,24 @@ public final class SanitizedConfiguration {
     return tasks;
   }
 
+  /**
+   * Determines whether this job is configured as a cron job.
+   *
+   * @return {@code true} if this is a cron job, otherwise {@code false}.
+   */
+  public boolean isCron() {
+    if (getJobConfig().isSetCronSchedule()) {
+      if (StringUtils.isEmpty(getJobConfig().getCronSchedule())) {
+        // TODO(ksweeney): Remove this in 0.7.0 (AURORA-423).
+        LOG.warning("Got service config with empty string cron schedule. aurora-0.7.x "
+            + "will interpret this as cron job and cause an error.");
+        return false;
+      }
+      return true;
+    }
+    return false;
+  }
+
   @Override
   public boolean equals(Object o) {
     if (!(o instanceof SanitizedConfiguration)) {

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java b/src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java
index cc08137..57d874b 100644
--- a/src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java
+++ b/src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java
@@ -97,7 +97,7 @@ class AuroraCronJob implements Job {
     private final Map<Integer, ITaskConfig> pendingTasks;
     private final Set<String> activeTaskIds;
 
-    private DeferredLaunch(Map<Integer, ITaskConfig> pendingTasks, Set<String> activeTaskIds) {
+    DeferredLaunch(Map<Integer, ITaskConfig> pendingTasks, Set<String> activeTaskIds) {
       this.pendingTasks = pendingTasks;
       this.activeTaskIds = activeTaskIds;
     }

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronLifecycle.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronLifecycle.java b/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronLifecycle.java
index 8f72a2d..02ba94a 100644
--- a/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronLifecycle.java
+++ b/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronLifecycle.java
@@ -32,6 +32,7 @@ import org.apache.aurora.scheduler.cron.SanitizedCronJob;
 import org.apache.aurora.scheduler.events.PubsubEvent;
 import org.apache.aurora.scheduler.storage.entities.IJobConfiguration;
 import org.quartz.Scheduler;
+import org.quartz.SchedulerException;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 
@@ -79,7 +80,7 @@ class CronLifecycle extends AbstractIdleService implements PubsubEvent.EventSubs
   }
 
   @Override
-  protected void startUp() throws Exception {
+  protected void startUp() throws SchedulerException {
     LOG.info("Starting Quartz cron scheduler" + scheduler.getSchedulerName() + ".");
     scheduler.start();
     RUNNING_FLAG.set(1);
@@ -104,7 +105,7 @@ class CronLifecycle extends AbstractIdleService implements PubsubEvent.EventSubs
   }
 
   @Override
-  protected void shutDown() throws Exception {
+  protected void shutDown() throws SchedulerException {
     LOG.info("Shutting down Quartz cron scheduler.");
     scheduler.shutdown();
     RUNNING_FLAG.set(0);

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronModule.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronModule.java b/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronModule.java
index 0094c88..88cb360 100644
--- a/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronModule.java
+++ b/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronModule.java
@@ -89,7 +89,7 @@ public class CronModule extends AbstractModule {
   }
 
   @Provides
-  private TimeZone provideTimeZone() {
+  TimeZone provideTimeZone() {
     TimeZone timeZone = TimeZone.getTimeZone(CRON_TIMEZONE.get());
     TimeZone systemTimeZone = TimeZone.getDefault();
     if (!timeZone.equals(systemTimeZone)) {
@@ -109,7 +109,7 @@ public class CronModule extends AbstractModule {
    */
   @Provides
   @Singleton
-  private static synchronized Scheduler provideScheduler(AuroraCronJobFactory jobFactory) {
+  static synchronized Scheduler provideScheduler(AuroraCronJobFactory jobFactory) {
     SimpleThreadPool threadPool = new SimpleThreadPool(NUM_THREADS.get(), Thread.NORM_PRIORITY);
     threadPool.setMakeThreadsDaemons(true);
 

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java b/src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java
index 06a5f8f..2afbef8 100644
--- a/src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java
+++ b/src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java
@@ -34,13 +34,13 @@ public interface PubsubEvent {
   /**
    * Interface with no functionality, but identifies a class as supporting task pubsub events.
    */
-  public interface EventSubscriber {
+  interface EventSubscriber {
   }
 
   /**
    * Event sent when tasks were deleted.
    */
-  public static class TasksDeleted implements PubsubEvent {
+  class TasksDeleted implements PubsubEvent {
     private final Set<IScheduledTask> tasks;
 
     public TasksDeleted(Set<IScheduledTask> tasks) {
@@ -76,8 +76,10 @@ public interface PubsubEvent {
 
   /**
    * Event sent when a task changed state.
+   * <p>
+   * This class is final as it should only be constructed through declared factory methods.
    */
-  public static final class TaskStateChange implements PubsubEvent {
+  final class TaskStateChange implements PubsubEvent {
     private final IScheduledTask task;
     private final Optional<ScheduleStatus> oldState;
 
@@ -156,7 +158,7 @@ public interface PubsubEvent {
   /**
    * Event sent when a host changed maintenance state.
    */
-  public static class HostMaintenanceStateChange implements PubsubEvent {
+  class HostMaintenanceStateChange implements PubsubEvent {
     private final HostStatus status;
 
     public HostMaintenanceStateChange(HostStatus status) {
@@ -186,7 +188,7 @@ public interface PubsubEvent {
   /**
    * Event sent when a scheduling assignment was vetoed.
    */
-  public static class Vetoed implements PubsubEvent {
+  class Vetoed implements PubsubEvent {
     private final String taskId;
     private final Set<Veto> vetoes;
 
@@ -220,7 +222,7 @@ public interface PubsubEvent {
     }
   }
 
-  public static class DriverRegistered implements PubsubEvent {
+  class DriverRegistered implements PubsubEvent {
     @Override
     public boolean equals(Object o) {
       return o != null && getClass().equals(o.getClass());
@@ -232,7 +234,7 @@ public interface PubsubEvent {
     }
   }
 
-  public static class DriverDisconnected implements PubsubEvent {
+  class DriverDisconnected implements PubsubEvent {
     @Override
     public boolean equals(Object o) {
       return o != null && getClass().equals(o.getClass());
@@ -244,7 +246,7 @@ public interface PubsubEvent {
     }
   }
 
-  public static class SchedulerActive implements PubsubEvent {
+  class SchedulerActive implements PubsubEvent {
     @Override
     public boolean equals(Object o) {
       return o != null && getClass().equals(o.getClass());

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java b/src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java
index d8daa68..b206830 100644
--- a/src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java
+++ b/src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java
@@ -32,7 +32,7 @@ public interface SchedulingFilter {
    * is only intended to be used for relative ranking of vetoes for determining which veto against
    * a scheduling assignment is 'weakest'.
    */
-  public static class Veto {
+  class Veto {
     public static final int MAX_SCORE = 1000;
 
     private final String reason;

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/http/JerseyTemplateServlet.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/http/JerseyTemplateServlet.java b/src/main/java/org/apache/aurora/scheduler/http/JerseyTemplateServlet.java
index c955640..96aec08 100644
--- a/src/main/java/org/apache/aurora/scheduler/http/JerseyTemplateServlet.java
+++ b/src/main/java/org/apache/aurora/scheduler/http/JerseyTemplateServlet.java
@@ -26,12 +26,14 @@ import org.antlr.stringtemplate.StringTemplate;
 
 /**
  * Base class for common functions needed in a jersey stringtemplate servlet.
+ *
+ * TODO(wfarner): This class should be composed rather than extended.  Turn it into a helper class.
  */
-abstract class JerseyTemplateServlet {
+class JerseyTemplateServlet {
 
   private final StringTemplateHelper templateHelper;
 
-  JerseyTemplateServlet(String templatePath) {
+  protected JerseyTemplateServlet(String templatePath) {
     templateHelper = new StringTemplateHelper(getClass(), templatePath, true);
   }
 

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java b/src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java
index 9cb85bb..070c7da 100644
--- a/src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java
+++ b/src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java
@@ -128,13 +128,13 @@ public class LeaderRedirect {
           .append(req.getScheme())
           .append("://")
           .append(target.getHostText())
-          .append(":")
+          .append(':')
           .append(target.getPort())
           .append(req.getRequestURI());
 
       String queryString = req.getQueryString();
       if (queryString != null) {
-        redirect.append("?").append(queryString);
+        redirect.append('?').append(queryString);
       }
 
       return Optional.of(redirect.toString());

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/http/Quotas.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/http/Quotas.java b/src/main/java/org/apache/aurora/scheduler/http/Quotas.java
index b516470..438d127 100644
--- a/src/main/java/org/apache/aurora/scheduler/http/Quotas.java
+++ b/src/main/java/org/apache/aurora/scheduler/http/Quotas.java
@@ -89,7 +89,7 @@ public class Quotas {
     private final long ramMb;
     private final long diskMb;
 
-    private ResourceAggregateBean(double cpu, long ramMb, long diskMb) {
+    ResourceAggregateBean(double cpu, long ramMb, long diskMb) {
       this.cpu = cpu;
       this.ramMb = ramMb;
       this.diskMb = diskMb;

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/http/Slaves.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/http/Slaves.java b/src/main/java/org/apache/aurora/scheduler/http/Slaves.java
index 56a8ce1..4c26db4 100644
--- a/src/main/java/org/apache/aurora/scheduler/http/Slaves.java
+++ b/src/main/java/org/apache/aurora/scheduler/http/Slaves.java
@@ -46,7 +46,7 @@ import static org.apache.aurora.scheduler.storage.Storage.Work;
 @Path("/slaves")
 public class Slaves extends JerseyTemplateServlet {
   private final String clusterName;
-  private Storage storage;
+  private final Storage storage;
 
   /**
    * Injected constructor.

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/http/StructDump.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/http/StructDump.java b/src/main/java/org/apache/aurora/scheduler/http/StructDump.java
index 7bf2fba..a7fbcbf 100644
--- a/src/main/java/org/apache/aurora/scheduler/http/StructDump.java
+++ b/src/main/java/org/apache/aurora/scheduler/http/StructDump.java
@@ -123,10 +123,10 @@ public class StructDump extends JerseyTemplateServlet {
       public void execute(StringTemplate template) {
         template.setAttribute("id", id);
         Optional<? extends TBase<?, ?>> struct = storage.weaklyConsistentRead(work);
-        if (!struct.isPresent()) {
-          template.setAttribute("exception", "Entity not found");
-        } else {
+        if (struct.isPresent()) {
           template.setAttribute("structPretty", Util.prettyPrint(struct.get()));
+        } else {
+          template.setAttribute("exception", "Entity not found");
         }
       }
     });

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/local/IsolatedSchedulerModule.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/local/IsolatedSchedulerModule.java b/src/main/java/org/apache/aurora/scheduler/local/IsolatedSchedulerModule.java
index a9d5f8f..e9d9bc4 100644
--- a/src/main/java/org/apache/aurora/scheduler/local/IsolatedSchedulerModule.java
+++ b/src/main/java/org/apache/aurora/scheduler/local/IsolatedSchedulerModule.java
@@ -14,16 +14,12 @@
 package org.apache.aurora.scheduler.local;
 
 import java.util.List;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.Future;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicLong;
-import java.util.logging.Level;
 import java.util.logging.Logger;
 
-import javax.annotation.Nullable;
 import javax.inject.Inject;
 import javax.inject.Provider;
 import javax.inject.Singleton;
@@ -34,7 +30,6 @@ import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.eventbus.Subscribe;
-import com.google.common.util.concurrent.ThreadFactoryBuilder;
 import com.google.inject.AbstractModule;
 import com.twitter.common.application.ShutdownRegistry;
 import com.twitter.common.base.Command;
@@ -56,6 +51,7 @@ import org.apache.aurora.gen.TaskConfig;
 import org.apache.aurora.gen.comm.DeletedTasks;
 import org.apache.aurora.gen.comm.SchedulerMessage;
 import org.apache.aurora.scheduler.DriverFactory;
+import org.apache.aurora.scheduler.base.AsyncUtil;
 import org.apache.aurora.scheduler.base.JobKeys;
 import org.apache.aurora.scheduler.configuration.ConfigurationManager;
 import org.apache.aurora.scheduler.configuration.Resources;
@@ -136,24 +132,8 @@ public class IsolatedSchedulerModule extends AbstractModule {
     }
 
     private static ScheduledExecutorService createThreadPool(ShutdownRegistry shutdownRegistry) {
-      final ScheduledThreadPoolExecutor executor = new ScheduledThreadPoolExecutor(
-          1,
-          new ThreadFactoryBuilder().setDaemon(true).setNameFormat("TaskScheduler-%d").build()) {
-
-        @Override
-        protected void afterExecute(Runnable runnable, @Nullable Throwable throwable) {
-          if (throwable != null) {
-            LOG.log(Level.WARNING, "Error: " + throwable, throwable);
-          } else if (runnable instanceof Future) {
-            Future<?> future = (Future<?>) runnable;
-            try {
-              future.get();
-            } catch (InterruptedException | ExecutionException e) {
-              e.printStackTrace();
-            }
-          }
-        }
-      };
+      final ScheduledThreadPoolExecutor executor =
+          AsyncUtil.loggingScheduledExecutor(1, "TaskScheduler-%d", LOG);
       Stats.exportSize("schedule_queue_size", executor.getQueue());
       shutdownRegistry.addAction(new Command() {
         @Override

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/quota/QuotaCheckResult.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/quota/QuotaCheckResult.java b/src/main/java/org/apache/aurora/scheduler/quota/QuotaCheckResult.java
index 396cecf..1a7b421 100644
--- a/src/main/java/org/apache/aurora/scheduler/quota/QuotaCheckResult.java
+++ b/src/main/java/org/apache/aurora/scheduler/quota/QuotaCheckResult.java
@@ -49,6 +49,10 @@ public class QuotaCheckResult {
     private Resource(String unit) {
       this.unit = unit;
     }
+
+    String getUnit() {
+      return unit;
+    }
   }
 
   private final Optional<String> details;
@@ -107,7 +111,7 @@ public class QuotaCheckResult {
           .append(" quota exceeded by ")
           .append(String.format("%.2f", b - a))
           .append(" ")
-          .append(resource.unit);
+          .append(resource.getUnit());
     }
 
     return result;

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/state/LockManagerImpl.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/state/LockManagerImpl.java b/src/main/java/org/apache/aurora/scheduler/state/LockManagerImpl.java
index 3390038..0568529 100644
--- a/src/main/java/org/apache/aurora/scheduler/state/LockManagerImpl.java
+++ b/src/main/java/org/apache/aurora/scheduler/state/LockManagerImpl.java
@@ -21,6 +21,7 @@ import com.google.common.base.Optional;
 import com.twitter.common.util.Clock;
 
 import org.apache.aurora.gen.Lock;
+import org.apache.aurora.gen.LockKey._Fields;
 import org.apache.aurora.scheduler.base.JobKeys;
 import org.apache.aurora.scheduler.storage.LockStore;
 import org.apache.aurora.scheduler.storage.Storage;
@@ -121,11 +122,8 @@ class LockManagerImpl implements LockManager {
   }
 
   private static String formatLockKey(ILockKey lockKey) {
-    switch (lockKey.getSetField()) {
-      case JOB:
-        return JobKeys.canonicalString(lockKey.getJob());
-      default:
-        return "Unknown lock key type: " + lockKey.getSetField();
-    }
+    return lockKey.getSetField() == _Fields.JOB
+        ? JobKeys.canonicalString(lockKey.getJob())
+        : "Unknown lock key type: " + lockKey.getSetField();
   }
 }

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java b/src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java
index 46446a9..44ced61 100644
--- a/src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java
+++ b/src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java
@@ -49,7 +49,6 @@ import org.apache.aurora.scheduler.storage.entities.IJobConfiguration;
 import org.apache.aurora.scheduler.storage.entities.IJobKey;
 import org.apache.aurora.scheduler.storage.entities.IScheduledTask;
 import org.apache.aurora.scheduler.storage.entities.ITaskConfig;
-import org.apache.commons.lang.StringUtils;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 
@@ -112,19 +111,6 @@ class SchedulerCoreImpl implements SchedulerCore {
     return hasActiveTasks || cronJobManager.hasJob(job.getKey());
   }
 
-  private static boolean isCron(SanitizedConfiguration config) {
-    if (!config.getJobConfig().isSetCronSchedule()) {
-      return false;
-    } else if (StringUtils.isEmpty(config.getJobConfig().getCronSchedule())) {
-      // TODO(ksweeney): Remove this in 0.7.0 (AURORA-423).
-      LOG.warning("Got service config with empty string cron schedule. aurora-0.7.x "
-          + "will interpret this as cron job and cause an error.");
-      return false;
-    } else {
-      return true;
-    }
-  }
-
   @Override
   public synchronized void createJob(final SanitizedConfiguration sanitizedConfiguration)
       throws ScheduleException {
@@ -140,7 +126,7 @@ class SchedulerCoreImpl implements SchedulerCore {
 
         validateTaskLimits(job.getTaskConfig(), job.getInstanceCount());
         // TODO(mchucarroll): deprecate cron as a part of create/kill job.(AURORA-454)
-        if (isCron(sanitizedConfiguration)) {
+        if (sanitizedConfiguration.isCron()) {
           try {
             LOG.warning("Deprecated behavior: scheduling job " + job.getKey()
                 + " with cron via createJob (AURORA_454)");

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/AttributeStore.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/AttributeStore.java b/src/main/java/org/apache/aurora/scheduler/storage/AttributeStore.java
index 616bdc4..6aa3e1b 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/AttributeStore.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/AttributeStore.java
@@ -46,7 +46,7 @@ public interface AttributeStore {
    * Attributes are considered mostly ephemeral and extremely low risk when inconsistency
    * is present.
    */
-  public interface Mutable extends AttributeStore {
+  interface Mutable extends AttributeStore {
 
     /**
      * Deletes all attributes in the store.
@@ -72,7 +72,7 @@ public interface AttributeStore {
     boolean setMaintenanceMode(String host, MaintenanceMode mode);
   }
 
-  public static final class Util {
+  final class Util {
     private Util() {
     }
 
@@ -90,5 +90,4 @@ public interface AttributeStore {
           ? attributes.get().getAttributes() : ImmutableList.<Attribute>of();
     }
   }
-
 }

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/JobStore.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/JobStore.java b/src/main/java/org/apache/aurora/scheduler/storage/JobStore.java
index dcd0ab0..ad0d67a 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/JobStore.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/JobStore.java
@@ -50,7 +50,7 @@ public interface JobStore {
    */
   Set<String> fetchManagerIds();
 
-  public interface Mutable extends JobStore {
+  interface Mutable extends JobStore {
     /**
      * Saves the job configuration for a job that has been accepted by the scheduler. Acts as an
      * update if the managerId already exists.

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/LockStore.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/LockStore.java b/src/main/java/org/apache/aurora/scheduler/storage/LockStore.java
index 4e2371c..596a378 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/LockStore.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/LockStore.java
@@ -39,7 +39,7 @@ public interface LockStore {
    */
   Optional<ILock> fetchLock(ILockKey lockKey);
 
-  public interface Mutable extends LockStore {
+  interface Mutable extends LockStore {
     /**
      * Saves a new lock or overwrites the existing one with same LockKey.
      *

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/QuotaStore.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/QuotaStore.java b/src/main/java/org/apache/aurora/scheduler/storage/QuotaStore.java
index 0270e58..688eb56 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/QuotaStore.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/QuotaStore.java
@@ -38,7 +38,7 @@ public interface QuotaStore {
    */
   Map<String, IResourceAggregate> fetchQuotas();
 
-  public interface Mutable extends QuotaStore {
+  interface Mutable extends QuotaStore {
 
     /**
      * Deletes all quotas.

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/ReadWriteLockManager.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/ReadWriteLockManager.java b/src/main/java/org/apache/aurora/scheduler/storage/ReadWriteLockManager.java
index c16f70d..fa49e22 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/ReadWriteLockManager.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/ReadWriteLockManager.java
@@ -41,6 +41,10 @@ public class ReadWriteLockManager {
     private LockType(LockMode mode) {
       this.mode = mode;
     }
+
+    LockMode getMode() {
+      return mode;
+    }
   }
 
   private static class LockState {
@@ -53,14 +57,14 @@ public class ReadWriteLockManager {
         initialLockMode = mode;
         stateChanged = true;
       }
-      if (initialLockMode == mode) {
+      if (initialLockMode.equals(mode)) {
         lockCount++;
       }
       return stateChanged;
     }
 
     private void lockReleased(LockMode mode) {
-      if (initialLockMode == mode) {
+      if (initialLockMode.equals(mode)) {
         lockCount--;
         if (lockCount == 0) {
           initialLockMode = LockMode.NONE;
@@ -95,7 +99,7 @@ public class ReadWriteLockManager {
       lock.writeLock().lock();
     }
 
-    return lockState.get().lockAcquired(type.mode);
+    return lockState.get().lockAcquired(type.getMode());
   }
 
   /**
@@ -112,7 +116,7 @@ public class ReadWriteLockManager {
       lock.writeLock().unlock();
     }
 
-    lockState.get().lockReleased(type.mode);
+    lockState.get().lockReleased(type.getMode());
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/SchedulerStore.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/SchedulerStore.java b/src/main/java/org/apache/aurora/scheduler/storage/SchedulerStore.java
index f6a992d..057a2e6 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/SchedulerStore.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/SchedulerStore.java
@@ -27,7 +27,7 @@ public interface SchedulerStore {
    */
   @Nullable String fetchFrameworkId();
 
-  public interface Mutable extends SchedulerStore {
+  interface Mutable extends SchedulerStore {
     /**
      * Stores the given framework id overwriting any previously saved id.
      *

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/Storage.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/Storage.java b/src/main/java/org/apache/aurora/scheduler/storage/Storage.java
index 768a821..bbbd7dc 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/Storage.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/Storage.java
@@ -252,7 +252,7 @@ public interface Storage {
   /**
    * Utility functions for interacting with a Storage instance.
    */
-  public final class Util {
+  final class Util {
 
     private Util() {
       // Utility class.

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java b/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java
index d507666..8c20ab6 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java
@@ -84,7 +84,9 @@ public final class StorageBackfill {
         // We want to keep exactly one task from this shard, so sort the IDs and keep the
         // highest (newest) in the hopes that it is legitimately running.
         String newestTask = Iterables.getLast(Sets.newTreeSet(activeTasksInShard));
-        if (!Tasks.id(task).equals(newestTask)) {
+        if (Tasks.id(task).equals(newestTask)) {
+          LOG.info("Retaining task " + Tasks.id(task));
+        } else {
           task.setStatus(ScheduleStatus.KILLED);
           task.addToTaskEvents(new TaskEvent(clock.nowMillis(), ScheduleStatus.KILLED)
               .setMessage("Killed duplicate shard."));
@@ -92,8 +94,6 @@ public final class StorageBackfill {
           // condition between the time the scheduler is actually available without hitting
           // IllegalStateException (see DriverImpl).
           // driver.killTask(Tasks.id(task));
-        } else {
-          LOG.info("Retaining task " + Tasks.id(task));
         }
       }
     }

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java b/src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java
index 40503b4..b76c937 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java
@@ -36,12 +36,12 @@ public interface TaskStore {
    */
   ImmutableSet<IScheduledTask> fetchTasks(Query.Builder query);
 
-  public interface Mutable extends TaskStore {
+  interface Mutable extends TaskStore {
 
     /**
      * A convenience interface to allow callers to more concisely implement a task mutation.
      */
-    public interface TaskMutation extends Function<IScheduledTask, IScheduledTask> {
+    interface TaskMutation extends Function<IScheduledTask, IScheduledTask> {
     }
 
     /**

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/backup/BackupModule.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/backup/BackupModule.java b/src/main/java/org/apache/aurora/scheduler/storage/backup/BackupModule.java
index 17b79c3..4e2fb8b 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/backup/BackupModule.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/backup/BackupModule.java
@@ -119,13 +119,13 @@ public class BackupModule extends PrivateModule {
   }
 
   @Provides
-  private File provideBackupDir() {
+  File provideBackupDir() {
     if (!unvalidatedBackupDir.exists()) {
-      if (!unvalidatedBackupDir.mkdirs()) {
+      if (unvalidatedBackupDir.mkdirs()) {
+        LOG.info("Created backup dir " + unvalidatedBackupDir.getPath() + ".");
+      } else {
         throw new IllegalArgumentException(
             "Unable to create backup dir " + unvalidatedBackupDir.getPath() + ".");
-      } else {
-        LOG.info("Created backup dir " + unvalidatedBackupDir.getPath() + ".");
       }
     }
 
@@ -138,7 +138,7 @@ public class BackupModule extends PrivateModule {
   }
 
   @Provides
-  private BackupConfig provideBackupConfig(File backupDir) {
+  BackupConfig provideBackupConfig(File backupDir) {
     return new BackupConfig(backupDir, MAX_SAVED_BACKUPS.get(), BACKUP_INTERVAL.get());
   }
 }

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/backup/Recovery.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/backup/Recovery.java b/src/main/java/org/apache/aurora/scheduler/storage/backup/Recovery.java
index e3c09e0..f1511b8 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/backup/Recovery.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/backup/Recovery.java
@@ -93,7 +93,7 @@ public interface Recovery {
    * Thrown when a recovery operation could not be completed due to internal errors or improper
    * invocation order.
    */
-  public static class RecoveryException extends Exception {
+  class RecoveryException extends Exception {
     RecoveryException(String message) {
       super(message);
     }

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/log/LogManager.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/log/LogManager.java b/src/main/java/org/apache/aurora/scheduler/storage/log/LogManager.java
index c31fe2d..496f13e 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/log/LogManager.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/log/LogManager.java
@@ -319,7 +319,7 @@ public final class LogManager {
       private final MessageDigest digest;
       private final int maxEntrySizeBytes;
 
-      private EntrySerializer(MessageDigest digest, Amount<Integer, Data> maxEntrySize) {
+      EntrySerializer(MessageDigest digest, Amount<Integer, Data> maxEntrySize) {
         this.digest = checkNotNull(digest);
         maxEntrySizeBytes = maxEntrySize.as(Data.BYTES);
       }
@@ -374,7 +374,7 @@ public final class LogManager {
           new Transaction().setSchemaVersion(storageConstants.CURRENT_SCHEMA_VERSION);
       private final AtomicBoolean committed = new AtomicBoolean(false);
 
-      private StreamTransaction() {
+      StreamTransaction() {
         // supplied by factory method
       }
 

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java b/src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java
index 899c61a..7fcc072 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java
@@ -343,7 +343,7 @@ public class LogStorage implements NonVolatileStorage, DistributedSnapshotStore
   }
 
   private static final class RecoveryFailedException extends SchedulerException {
-    private RecoveryFailedException(Throwable cause) {
+    RecoveryFailedException(Throwable cause) {
       super(cause);
     }
   }
@@ -451,10 +451,10 @@ public class LogStorage implements NonVolatileStorage, DistributedSnapshotStore
           try {
             snapshot();
           } catch (StorageException e) {
-            if (e.getCause() != null) {
-              LOG.log(Level.WARNING, e.getMessage(), e.getCause());
-            } else {
+            if (e.getCause() == null) {
               LOG.log(Level.WARNING, "StorageException when attempting to snapshot.", e);
+            } else {
+              LOG.log(Level.WARNING, e.getMessage(), e.getCause());
             }
           }
         }

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/log/testing/LogOpMatcher.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/log/testing/LogOpMatcher.java b/src/main/java/org/apache/aurora/scheduler/storage/log/testing/LogOpMatcher.java
index 87a442b..35e2e5a 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/log/testing/LogOpMatcher.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/log/testing/LogOpMatcher.java
@@ -69,7 +69,7 @@ public class LogOpMatcher implements IArgumentMatcher {
   public static final class StreamMatcher {
     private final Stream stream;
 
-    private StreamMatcher(Stream stream) {
+    StreamMatcher(Stream stream) {
       this.stream = Preconditions.checkNotNull(stream);
     }
 
@@ -106,6 +106,6 @@ public class LogOpMatcher implements IArgumentMatcher {
    */
   private static byte[] sameEntry(LogEntry entry) {
     EasyMock.reportMatcher(new LogOpMatcher(entry));
-    return null;
+    return new byte[] {};
   }
 }

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStore.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStore.java b/src/main/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStore.java
index 429691c..ff9e45c 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStore.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStore.java
@@ -51,11 +51,11 @@ class MemAttributeStore implements Mutable {
   @Override
   public boolean setMaintenanceMode(String host, MaintenanceMode mode) {
     HostAttributes stored = hostAttributes.get(host);
-    if (stored != null) {
+    if (stored == null) {
+      return false;
+    } else {
       stored.setMode(mode);
       return true;
-    } else {
-      return false;
     }
   }
 

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobStore.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobStore.java b/src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobStore.java
index 962861d..571636d 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobStore.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobStore.java
@@ -88,10 +88,10 @@ class MemJobStore implements JobStore.Mutable {
     checkNotNull(jobKey);
 
     Optional<Manager> manager = Optional.fromNullable(managers.getIfPresent(managerId));
-    if (!manager.isPresent()) {
-      return Optional.absent();
-    } else {
+    if (manager.isPresent()) {
       return Optional.fromNullable(manager.get().jobs.get(jobKey));
+    } else {
+      return Optional.absent();
     }
   }
 

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java b/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
index 6f3ebd3..29f64f7 100644
--- a/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
+++ b/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
@@ -67,6 +67,7 @@ import org.apache.aurora.gen.JobSummaryResult;
 import org.apache.aurora.gen.ListBackupsResult;
 import org.apache.aurora.gen.Lock;
 import org.apache.aurora.gen.LockKey;
+import org.apache.aurora.gen.LockKey._Fields;
 import org.apache.aurora.gen.LockValidation;
 import org.apache.aurora.gen.MaintenanceStatusResult;
 import org.apache.aurora.gen.PopulateJobResult;
@@ -252,19 +253,6 @@ class SchedulerThriftInterface implements AuroraAdmin.Iface {
     return response;
   }
 
-  private static boolean isCron(SanitizedConfiguration config) {
-    if (!config.getJobConfig().isSetCronSchedule()) {
-      return false;
-    } else if (StringUtils.isEmpty(config.getJobConfig().getCronSchedule())) {
-      // TODO(ksweeney): Remove this in 0.7.0 (AURORA-424).
-      LOG.warning("Got service config with empty string cron schedule. aurora-0.7.x "
-          + "will interpret this as cron job and cause an error.");
-      return false;
-    } else {
-      return true;
-    }
-  }
-
   @Override
   public Response scheduleCronJob(
       JobConfiguration mutableJob,
@@ -290,7 +278,7 @@ class SchedulerThriftInterface implements AuroraAdmin.Iface {
           ILockKey.build(LockKey.job(jobKey.newBuilder())),
           Optional.fromNullable(mutableLock).transform(ILock.FROM_BUILDER));
 
-      if (!isCron(sanitized)) {
+      if (!sanitized.isCron()) {
         LOG.info("Invalid attempt to schedule non-cron job "
             + sanitized.getJobConfig().getKey()
             + " with cron.");
@@ -870,16 +858,97 @@ class SchedulerThriftInterface implements AuroraAdmin.Iface {
         }
 
         Response resp = new Response();
-        if (!errors.isEmpty()) {
-          resp.setResponseCode(ResponseCode.WARNING).setMessage(Joiner.on(", ").join(errors));
-        } else {
+        if (errors.isEmpty()) {
           resp.setResponseCode(OK).setMessage("All rewrites completed successfully.");
+        } else {
+          resp.setResponseCode(ResponseCode.WARNING).setMessage(Joiner.on(", ").join(errors));
         }
         return resp;
       }
     });
   }
 
+  private Optional<String> rewriteJob(JobConfigRewrite jobRewrite, JobStore.Mutable jobStore) {
+    IJobConfiguration existingJob = IJobConfiguration.build(jobRewrite.getOldJob());
+    IJobConfiguration rewrittenJob;
+    Optional<String> error = Optional.absent();
+    try {
+      rewrittenJob = ConfigurationManager.validateAndPopulate(
+          IJobConfiguration.build(jobRewrite.getRewrittenJob()));
+    } catch (TaskDescriptionException e) {
+      // We could add an error here, but this is probably a hint of something wrong in
+      // the client that's causing a bad configuration to be applied.
+      throw Throwables.propagate(e);
+    }
+
+    if (existingJob.getKey().equals(rewrittenJob.getKey())) {
+      if (existingJob.getOwner().equals(rewrittenJob.getOwner())) {
+        Multimap<String, IJobConfiguration> matches = jobsByKey(jobStore, existingJob.getKey());
+        switch (matches.size()) {
+          case 0:
+            error = Optional.of(
+                "No jobs found for key " + JobKeys.canonicalString(existingJob.getKey()));
+            break;
+
+          case 1:
+            Map.Entry<String, IJobConfiguration> match =
+                Iterables.getOnlyElement(matches.entries());
+            IJobConfiguration storedJob = match.getValue();
+            if (storedJob.equals(existingJob)) {
+              jobStore.saveAcceptedJob(match.getKey(), rewrittenJob);
+            } else {
+              error = Optional.of(
+                  "CAS compare failed for " + JobKeys.canonicalString(storedJob.getKey()));
+            }
+            break;
+
+          default:
+            error = Optional.of("Multiple jobs found for key "
+                + JobKeys.canonicalString(existingJob.getKey()));
+        }
+      } else {
+        error = Optional.of("Disallowing rewrite attempting to change job owner.");
+      }
+    } else {
+      error = Optional.of("Disallowing rewrite attempting to change job key.");
+    }
+
+    return error;
+  }
+
+  private Optional<String> rewriteInstance(
+      InstanceConfigRewrite instanceRewrite,
+      MutableStoreProvider storeProvider) {
+
+    InstanceKey instanceKey = instanceRewrite.getInstanceKey();
+    Optional<String> error = Optional.absent();
+    Iterable<IScheduledTask> tasks = storeProvider.getTaskStore().fetchTasks(
+        Query.instanceScoped(IJobKey.build(instanceKey.getJobKey()),
+            instanceKey.getInstanceId())
+            .active());
+    Optional<IAssignedTask> task =
+        Optional.fromNullable(Iterables.getOnlyElement(tasks, null))
+            .transform(Tasks.SCHEDULED_TO_ASSIGNED);
+
+    if (task.isPresent()) {
+      if (task.get().getTask().newBuilder().equals(instanceRewrite.getOldTask())) {
+        ITaskConfig newConfiguration = ITaskConfig.build(
+            ConfigurationManager.applyDefaultsIfUnset(instanceRewrite.getRewrittenTask()));
+        boolean changed = storeProvider.getUnsafeTaskStore().unsafeModifyInPlace(
+            task.get().getTaskId(), newConfiguration);
+        if (!changed) {
+          error = Optional.of("Did not change " + task.get().getTaskId());
+        }
+      } else {
+        error = Optional.of("CAS compare failed for " + instanceKey);
+      }
+    } else {
+      error = Optional.of("No active task found for " + instanceKey);
+    }
+
+    return error;
+  }
+
   private Optional<String> rewriteConfig(
       ConfigRewrite command,
       MutableStoreProvider storeProvider) {
@@ -887,73 +956,11 @@ class SchedulerThriftInterface implements AuroraAdmin.Iface {
     Optional<String> error = Optional.absent();
     switch (command.getSetField()) {
       case JOB_REWRITE:
-        JobConfigRewrite jobRewrite = command.getJobRewrite();
-        IJobConfiguration existingJob = IJobConfiguration.build(jobRewrite.getOldJob());
-        IJobConfiguration rewrittenJob;
-        try {
-          rewrittenJob = ConfigurationManager.validateAndPopulate(
-              IJobConfiguration.build(jobRewrite.getRewrittenJob()));
-        } catch (TaskDescriptionException e) {
-          // We could add an error here, but this is probably a hint of something wrong in
-          // the client that's causing a bad configuration to be applied.
-          throw Throwables.propagate(e);
-        }
-        if (!existingJob.getKey().equals(rewrittenJob.getKey())) {
-          error = Optional.of("Disallowing rewrite attempting to change job key.");
-        } else if (!existingJob.getOwner().equals(rewrittenJob.getOwner())) {
-          error = Optional.of("Disallowing rewrite attempting to change job owner.");
-        } else {
-          JobStore.Mutable jobStore = storeProvider.getJobStore();
-          Multimap<String, IJobConfiguration> matches =
-              jobsByKey(jobStore, existingJob.getKey());
-          switch (matches.size()) {
-            case 0:
-              error = Optional.of(
-                  "No jobs found for key " + JobKeys.canonicalString(existingJob.getKey()));
-              break;
-
-            case 1:
-              Map.Entry<String, IJobConfiguration> match =
-                  Iterables.getOnlyElement(matches.entries());
-              IJobConfiguration storedJob = match.getValue();
-              if (!storedJob.equals(existingJob)) {
-                error = Optional.of(
-                    "CAS compare failed for " + JobKeys.canonicalString(storedJob.getKey()));
-              } else {
-                jobStore.saveAcceptedJob(match.getKey(), rewrittenJob);
-              }
-              break;
-
-            default:
-              error = Optional.of("Multiple jobs found for key "
-                  + JobKeys.canonicalString(existingJob.getKey()));
-          }
-        }
+        error = rewriteJob(command.getJobRewrite(), storeProvider.getJobStore());
         break;
 
       case INSTANCE_REWRITE:
-        InstanceConfigRewrite instanceRewrite = command.getInstanceRewrite();
-        InstanceKey instanceKey = instanceRewrite.getInstanceKey();
-        Iterable<IScheduledTask> tasks = storeProvider.getTaskStore().fetchTasks(
-            Query.instanceScoped(IJobKey.build(instanceKey.getJobKey()),
-                instanceKey.getInstanceId())
-                .active());
-        Optional<IAssignedTask> task =
-            Optional.fromNullable(Iterables.getOnlyElement(tasks, null))
-                .transform(Tasks.SCHEDULED_TO_ASSIGNED);
-        if (!task.isPresent()) {
-          error = Optional.of("No active task found for " + instanceKey);
-        } else if (!task.get().getTask().newBuilder().equals(instanceRewrite.getOldTask())) {
-          error = Optional.of("CAS compare failed for " + instanceKey);
-        } else {
-          ITaskConfig newConfiguration = ITaskConfig.build(
-              ConfigurationManager.applyDefaultsIfUnset(instanceRewrite.getRewrittenTask()));
-          boolean changed = storeProvider.getUnsafeTaskStore().unsafeModifyInPlace(
-              task.get().getTaskId(), newConfiguration);
-          if (!changed) {
-            error = Optional.of("Did not change " + task.get().getTaskId());
-          }
-        }
+        error = rewriteInstance(command.getInstanceRewrite(), storeProvider);
         break;
 
       default:
@@ -1006,12 +1013,11 @@ class SchedulerThriftInterface implements AuroraAdmin.Iface {
   }
 
   private String getRoleFromLockKey(ILockKey lockKey) {
-    switch (lockKey.getSetField()) {
-      case JOB:
-        JobKeys.assertValid(lockKey.getJob());
-        return lockKey.getJob().getRole();
-      default:
-        throw new IllegalArgumentException("Unhandled LockKey: " + lockKey.getSetField());
+    if (lockKey.getSetField() == _Fields.JOB) {
+      JobKeys.assertValid(lockKey.getJob());
+      return lockKey.getJob().getRole();
+    } else {
+      throw new IllegalArgumentException("Unhandled LockKey: " + lockKey.getSetField());
     }
   }
 

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptor.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptor.java b/src/main/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptor.java
index 8eb52dd..0c1477c 100644
--- a/src/main/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptor.java
+++ b/src/main/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptor.java
@@ -33,13 +33,13 @@ public class FeatureToggleInterceptor implements MethodInterceptor {
   @Override
   public Object invoke(MethodInvocation invocation) throws Throwable {
     Method method = invocation.getMethod();
-    if (!allowMethod.apply(method)) {
+    if (allowMethod.apply(method)) {
+      return invocation.proceed();
+    } else {
       return Interceptors.properlyTypedResponse(
           method,
           ResponseCode.ERROR,
           "The " + method.getName() + " feature is currently disabled on this scheduler.");
-    } else {
-      return invocation.proceed();
     }
   }
 }


Mime
View raw message