Merge pull request #791 from Expensify/revert-787-ionatan_retry_updat…

…e_priority Revert "Allow to update priority on job retry"
Expensify · May 14, 2020 · 59c4889 · 59c4889
2 parents 5fccd02 + 81c3b1f
commit 59c4889
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 26 deletions.
diff --git a/plugins/Jobs.cpp b/plugins/Jobs.cpp
@@ -898,12 +898,11 @@ void BedrockJobsCommand::process(SQLite& db) {
         //     interrupted in a non-fatal way.
         //
         //     Parameters:
-        //     - jobID       - ID of the job to requeue
-        //     - delay       - Number of seconds to wait before retrying
-        //     - nextRun     - datetime of next scheduled run
-        //     - name        - An arbitrary string identifier (case insensitive)
-        //     - data        - Data to associate with this job
-        //     - jobPriority - The new priority to set for this job
+        //     - jobID   - ID of the job to requeue
+        //     - delay   - Number of seconds to wait before retrying
+        //     - nextRun - datetime of next scheduled run
+        //     - name    - An arbitrary string identifier (case insensitive)
+        //     - data    - Data to associate with this job
         //
         // - FinishJob( jobID, [data] )
         //
@@ -1006,20 +1005,11 @@ void BedrockJobsCommand::process(SQLite& db) {
             return;
         }
 
-        // If this is RetryJob and we want to update the name and/or priority, let's do that
+        // If this is RetryJob and we want to update the name, let's do that
         const string& name = request["name"];
-        if (SIEquals(requestVerb, "RetryJob")) {
-            if (!name.empty() || request.isSet("jobPriority")) {
-                if (request.isSet("jobPriority")) {
-                    _validatePriority(request.calc64("jobPriority"));
-                }
-                bool success = db.writeIdempotent("UPDATE jobs SET " +
-                                                  (!name.empty() ? "name=" + SQ(name) + ", " : "") +
-                                                  (request.isSet("jobPriority") ? "priority=" + SQ(request["jobPriority"]) : "") + " "
-                                                  "WHERE jobID=" + SQ(jobID) + ";");
-                if (!success) {
-                    STHROW("502 Failed to update job name/priority");
-                }
+        if (!name.empty() && SIEquals(requestVerb, "RetryJob")) {
+            if (!db.writeIdempotent("UPDATE jobs SET name=" + SQ(name) + " WHERE jobID=" + SQ(jobID) + ";")) {
+                STHROW("502 Failed to update job name");
             }
         }
 

diff --git a/test/tests/jobs/RetryJobTest.cpp b/test/tests/jobs/RetryJobTest.cpp
@@ -16,7 +16,7 @@ struct RetryJobTest : tpunit::TestFixture {
                               TEST(RetryJobTest::hasRepeat),
                               TEST(RetryJobTest::inRunqueuedState),
                               TEST(RetryJobTest::simplyRetryWithNextRun),
-                              TEST(RetryJobTest::changeNameAndPriority),
+                              TEST(RetryJobTest::changeName),
                               TEST(RetryJobTest::hasRepeatWithNextRun),
                               TEST(RetryJobTest::hasRepeatWithDelay),
                               TEST(RetryJobTest::hasDelayAndNextRun),
@@ -388,12 +388,11 @@ struct RetryJobTest : tpunit::TestFixture {
         ASSERT_EQUAL(result[0][0], nextRun);
     }
 
-    // Update the name and priority
-    void changeNameAndPriority() {
+    // Update the name
+    void changeName() {
         // Create the job
         SData command("CreateJob");
         command["name"] = "job";
-        command["jobPriority"] = "500";
         STable response = tester->executeWaitVerifyContentTable(command);
         string jobID = response["jobID"];
 
@@ -408,15 +407,13 @@ struct RetryJobTest : tpunit::TestFixture {
         command.methodLine = "RetryJob";
         command["jobID"] = jobID;
         command["name"] = "newName";
-        command["jobPriority"] = "1000";
         command["nextRun"] = getTimeInFuture(10);
         tester->executeWaitVerifyContent(command);
 
         // Confirm the data updated
         SQResult result;
-        tester->readDB("SELECT name, priority FROM jobs WHERE jobID = " + jobID + ";", result);
+        tester->readDB("SELECT name FROM jobs WHERE jobID = " + jobID + ";", result);
         ASSERT_EQUAL(result[0][0], "newName");
-        ASSERT_EQUAL(result[0][1], "1000");
     }
 
     // Repeat should take precedence over nextRun