Fix a backfill issue for job ids containing a comma (#418)

The backfilled jobs list is now sent as a real list instead of a string with comma separated ids
criteo · Jul 10, 2019 · 11d4a58 · 11d4a58
1 parent 44b41e7
commit 11d4a58
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 19 deletions.
diff --git a/timeseries/src/main/javascript/app/menu/MenuHeader.js b/timeseries/src/main/javascript/app/menu/MenuHeader.js
@@ -25,7 +25,9 @@ const MenuHeader = ({
   projectVersion
 }: Props) => (
   <Link className={classNames(classes.main, className)} href="/">
-    <span title={projectName} className={classes.projectName}>{projectName}</span>
+    <span title={projectName} className={classes.projectName}>
+      {projectName}
+    </span>
     {projectVersion && (
       <span className={classes.projectVersion}>{projectVersion}</span>
     )}

diff --git a/timeseries/src/main/javascript/app/pages/BackfillCreate.js b/timeseries/src/main/javascript/app/pages/BackfillCreate.js
@@ -151,7 +151,7 @@ class BackfillCreate extends React.Component<Props> {
       body: JSON.stringify({
         name: name,
         description: description || "",
-        jobs: jobs.join(","),
+        jobs: jobs,
         priority: priority,
         startDate: start.toISOString(),
         endDate: end.toISOString()

diff --git a/timeseries/src/main/scala/com/criteo/cuttle/timeseries/Internal.scala b/timeseries/src/main/scala/com/criteo/cuttle/timeseries/Internal.scala
@@ -39,7 +39,7 @@ private[timeseries] object BackfillCreate {
 private[timeseries] case class BackfillCreate(
   name: String,
   description: String,
-  jobs: String,
+  jobs: List[String],
   startDate: Instant,
   endDate: Instant,
   priority: Int

diff --git a/timeseries/src/main/scala/com/criteo/cuttle/timeseries/TimeSeriesApp.scala b/timeseries/src/main/scala/com/criteo/cuttle/timeseries/TimeSeriesApp.scala
@@ -933,22 +933,25 @@ private[timeseries] case class TimeSeriesApp(project: CuttleProject,
                          |""".stripMargin
                   )),
               backfill => {
-                val jobIdsToBackfill = backfill.jobs.split(",").toSet
-                scheduler
-                  .backfillJob(
-                    backfill.name,
-                    backfill.description,
-                    jobs.all.filter(job => jobIdsToBackfill.contains(job.id)),
-                    backfill.startDate,
-                    backfill.endDate,
-                    backfill.priority,
-                    executor.runningState,
-                    xa
-                  )
-                  .map {
-                    case Right(_)     => Ok("ok".asJson)
-                    case Left(errors) => BadRequest(errors)
-                  }
+                val jobIdsToBackfill = backfill.jobs.toSet
+                jobIdsToBackfill.partition(j => jobs.all.map(_.id).contains(j)) match {
+                  case (_, r) if !r.isEmpty => BadRequest(s"Contains unknown job ids: ${r.map(s => s"'$s'").mkString(",")}")
+                  case (filtered, _) => scheduler
+                    .backfillJob(
+                      backfill.name,
+                      backfill.description,
+                      jobs.all.filter(j => filtered.contains(j.id)),
+                      backfill.startDate,
+                      backfill.endDate,
+                      backfill.priority,
+                      executor.runningState,
+                      xa
+                    )
+                    .map {
+                      case Right(_)     => Ok("ok".asJson)
+                      case Left(errors) => BadRequest(errors)
+                    }
+                }
               }
             )
         )