aurora-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From wfar...@apache.org
Subject incubator-aurora git commit: Reject jobs containing an empty cron schedule.
Date Wed, 03 Dec 2014 01:44:42 GMT
Repository: incubator-aurora
Updated Branches:
  refs/heads/master 552ae93d9 -> f9837dae8


Reject jobs containing an empty cron schedule.

Bugs closed: AURORA-424

Reviewed at https://reviews.apache.org/r/28571/


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

Branch: refs/heads/master
Commit: f9837dae852f56e9428961953fac1e99dccfaa2d
Parents: 552ae93
Author: Bill Farner <wfarner@apache.org>
Authored: Tue Dec 2 17:42:37 2014 -0800
Committer: Bill Farner <wfarner@apache.org>
Committed: Tue Dec 2 17:42:37 2014 -0800

----------------------------------------------------------------------
 .../configuration/SanitizedConfiguration.java        | 15 +--------------
 .../aurora/scheduler/cron/SanitizedCronJob.java      |  8 +++++---
 .../thrift/SchedulerThriftInterfaceTest.java         | 12 ++++--------
 3 files changed, 10 insertions(+), 25 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/f9837dae/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 fe3ad4c..05981b9 100644
--- a/src/main/java/org/apache/aurora/scheduler/configuration/SanitizedConfiguration.java
+++ b/src/main/java/org/apache/aurora/scheduler/configuration/SanitizedConfiguration.java
@@ -14,11 +14,9 @@
 package org.apache.aurora.scheduler.configuration;
 
 import java.util.Set;
-import java.util.logging.Logger;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Objects;
-import com.google.common.base.Strings;
 import com.google.common.collect.ContiguousSet;
 import com.google.common.collect.DiscreteDomain;
 import com.google.common.collect.Range;
@@ -31,8 +29,6 @@ import org.apache.aurora.scheduler.storage.entities.IJobConfiguration;
  */
 public final class SanitizedConfiguration {
 
-  private static final Logger LOG = Logger.getLogger(SanitizedConfiguration.class.getName());
-
   private final IJobConfiguration sanitized;
   private final Set<Integer> instanceIds;
 
@@ -77,16 +73,7 @@ public final class SanitizedConfiguration {
    * @return {@code true} if this is a cron job, otherwise {@code false}.
    */
   public boolean isCron() {
-    if (getJobConfig().isSetCronSchedule()) {
-      if (Strings.isNullOrEmpty(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;
+    return getJobConfig().isSetCronSchedule();
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/f9837dae/src/main/java/org/apache/aurora/scheduler/cron/SanitizedCronJob.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/cron/SanitizedCronJob.java b/src/main/java/org/apache/aurora/scheduler/cron/SanitizedCronJob.java
index 0e35cfb..46e89cd 100644
--- a/src/main/java/org/apache/aurora/scheduler/cron/SanitizedCronJob.java
+++ b/src/main/java/org/apache/aurora/scheduler/cron/SanitizedCronJob.java
@@ -15,12 +15,12 @@ package org.apache.aurora.scheduler.cron;
 
 import javax.annotation.Nullable;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Objects;
 import com.google.common.base.Optional;
 import com.google.common.base.Strings;
 
 import org.apache.aurora.gen.CronCollisionPolicy;
-import org.apache.aurora.scheduler.base.JobKeys;
 import org.apache.aurora.scheduler.configuration.ConfigurationManager;
 import org.apache.aurora.scheduler.configuration.SanitizedConfiguration;
 import org.apache.aurora.scheduler.storage.entities.IJobConfiguration;
@@ -31,6 +31,9 @@ import static java.util.Objects.requireNonNull;
  * Used by functions that expect field validation before being called.
  */
 public final class SanitizedCronJob {
+  @VisibleForTesting
+  public static final String NO_CRON_SCHEDULE = "Job has an empty cron schedule.";
+
   private final SanitizedConfiguration config;
   private final CrontabEntry crontabEntry;
 
@@ -43,8 +46,7 @@ public final class SanitizedCronJob {
   private SanitizedCronJob(SanitizedConfiguration config) throws CronException {
     final IJobConfiguration job = config.getJobConfig();
     if (!hasCronSchedule(job)) {
-      throw new CronException(String.format(
-          "Not a valid cron job, %s has no cron schedule", JobKeys.canonicalString(job.getKey())));
+      throw new CronException(NO_CRON_SCHEDULE);
     }
 
     Optional<CrontabEntry> entry = CrontabEntry.tryParse(job.getCronSchedule());

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/f9837dae/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
b/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
index b28b3ae..d687f57 100644
--- a/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
@@ -410,8 +410,7 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
   }
 
   @Test
-  public void testCreateCronJobEmptyCronSchedule() throws Exception {
-    // TODO(maxim): Deprecate this as part of AURORA-423.
+  public void testRejectCronJobEmptyCronSchedule() throws Exception {
     IJobConfiguration job = IJobConfiguration.build(makeProdJob().setCronSchedule(""));
     SanitizedConfiguration sanitized = SanitizedConfiguration.fromUnsanitized(job);
     expectAuth(ROLE, true);
@@ -423,14 +422,11 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
     expect(quotaManager.checkInstanceAddition(sanitized.getJobConfig().getTaskConfig(), 1))
         .andReturn(ENOUGH_QUOTA);
 
-    stateManager.insertPendingTasks(
-        storageUtil.mutableStoreProvider,
-        sanitized.getJobConfig().getTaskConfig(),
-        sanitized.getInstanceIds());
-
     control.replay();
 
-    assertOkResponse(thrift.createJob(job.newBuilder(), LOCK.newBuilder(), SESSION));
+    assertEquals(
+        invalidResponse(SanitizedCronJob.NO_CRON_SCHEDULE),
+        thrift.createJob(job.newBuilder(), LOCK.newBuilder(), SESSION));
   }
 
   @Test


Mime
View raw message