You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@aurora.apache.org by wf...@apache.org on 2014/12/03 02:44:42 UTC
incubator-aurora git commit: Reject jobs containing an empty cron
schedule.
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 <wf...@apache.org>
Authored: Tue Dec 2 17:42:37 2014 -0800
Committer: Bill Farner <wf...@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