From c73d154084f678e02d6765d21c4972e48194ecc1 Mon Sep 17 00:00:00 2001
From: Lai Jiang <jianglai@google.com>
Date: Thu, 21 May 2020 11:40:45 -0400
Subject: [PATCH] Do not enqueue update snapshot task if import fails (#578)

If the import from Datastore to BigQuery fails, there is no point
enqueuing a job to update the snapshot view.

Also when there's an error updating the snapshot view, log it at severe
level. The HTTP exception thrown is logged at info and triggers a retry
implicitly. I'm not sure if we want this behavior though. Do we want to
retry upon snapshot updating failures? Unless the failurs are transient,
retrying doesn't help. In our case the failure (End of time out of range
in Standard SQL) is not transient.
---
 .../java/google/registry/export/BigqueryPollJobAction.java  | 6 ++++--
 .../google/registry/export/BigqueryPollJobActionTest.java   | 2 ++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/core/src/main/java/google/registry/export/BigqueryPollJobAction.java b/core/src/main/java/google/registry/export/BigqueryPollJobAction.java
index e662b23b6..a5a4aebbf 100644
--- a/core/src/main/java/google/registry/export/BigqueryPollJobAction.java
+++ b/core/src/main/java/google/registry/export/BigqueryPollJobAction.java
@@ -73,8 +73,10 @@ public class BigqueryPollJobAction implements Runnable {
 
   @Override
   public void run() {
-    checkJobOutcome();  // Throws a NotModifiedException if the job hasn't completed.
-    if (payload == null || payload.length == 0) {
+    boolean jobOutcome =
+        checkJobOutcome(); // Throws a NotModifiedException if the job hasn't completed.
+    // If the job failed, do not enqueue the next step.
+    if (!jobOutcome || payload == null || payload.length == 0) {
       return;
     }
     // If there is a payload, it's a chained task, so enqueue it.
diff --git a/core/src/test/java/google/registry/export/BigqueryPollJobActionTest.java b/core/src/test/java/google/registry/export/BigqueryPollJobActionTest.java
index 998a088d9..add6ce8d0 100644
--- a/core/src/test/java/google/registry/export/BigqueryPollJobActionTest.java
+++ b/core/src/test/java/google/registry/export/BigqueryPollJobActionTest.java
@@ -17,6 +17,7 @@ package google.registry.export;
 import static com.google.appengine.api.taskqueue.QueueFactory.getQueue;
 import static com.google.common.collect.Iterables.getOnlyElement;
 import static com.google.common.truth.Truth.assertThat;
+import static google.registry.testing.TaskQueueHelper.assertNoTasksEnqueued;
 import static google.registry.testing.TaskQueueHelper.assertTasksEnqueued;
 import static google.registry.testing.TestLogHandlerUtils.assertLogMessage;
 import static java.nio.charset.StandardCharsets.UTF_8;
@@ -174,6 +175,7 @@ public class BigqueryPollJobActionTest {
     action.run();
     assertLogMessage(
         logHandler, SEVERE, String.format("Bigquery job failed - %s:%s", PROJECT_ID, JOB_ID));
+    assertNoTasksEnqueued(CHAINED_QUEUE_NAME);
   }
 
   @Test