Github Store should not error if the file cannot be found. (#4455)

Fixes #4411. There are two cases here: - No internet access - The file cannot be found. In both cases, we return an empty list instead. This turns this operation into a noop and Airbyte then uses the list bundled with the current version.
airbytehq · Jul 2, 2021 · 436471c · 436471c
1 parent 5d56d00
commit 436471c
Show file tree

Hide file tree

Showing 6 changed files with 82 additions and 22 deletions.
diff --git a/airbyte-server/src/main/java/io/airbyte/server/handlers/DestinationDefinitionsHandler.java b/airbyte-server/src/main/java/io/airbyte/server/handlers/DestinationDefinitionsHandler.java
@@ -42,14 +42,17 @@
 import java.io.IOException;
 import java.net.URI;
 import java.net.URISyntaxException;
-import java.util.Collections;
 import java.util.List;
 import java.util.UUID;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class DestinationDefinitionsHandler {
 
+  private static final Logger LOGGER = LoggerFactory.getLogger(DestinationDefinitionsHandler.class);
+
   private final DockerImageValidator imageValidator;
   private final ConfigRepository configRepository;
   private final Supplier<UUID> uuidSupplier;
@@ -108,8 +111,6 @@ public DestinationDefinitionReadList listLatestDestinationDefinitions() {
   private List<StandardDestinationDefinition> getLatestDestinations() {
     try {
       return githubStore.getLatestDestinations();
-    } catch (IOException e) {
-      return Collections.emptyList();
     } catch (InterruptedException e) {
       throw new InternalServerKnownException("Request to retrieve latest destination definitions failed", e);
     }

diff --git a/airbyte-server/src/main/java/io/airbyte/server/handlers/SourceDefinitionsHandler.java b/airbyte-server/src/main/java/io/airbyte/server/handlers/SourceDefinitionsHandler.java
@@ -42,7 +42,6 @@
 import java.io.IOException;
 import java.net.URI;
 import java.net.URISyntaxException;
-import java.util.Collections;
 import java.util.List;
 import java.util.UUID;
 import java.util.function.Supplier;
@@ -103,14 +102,12 @@ private static SourceDefinitionReadList toSourceDefinitionReadList(List<Standard
   }
 
   public SourceDefinitionReadList listLatestSourceDefinitions() {
-    return toSourceDefinitionReadList(getLatestDestinations());
+    return toSourceDefinitionReadList(getLatestSources());
   }
 
-  private List<StandardSourceDefinition> getLatestDestinations() {
+  private List<StandardSourceDefinition> getLatestSources() {
     try {
       return githubStore.getLatestSources();
-    } catch (IOException e) {
-      return Collections.emptyList();
     } catch (InterruptedException e) {
       throw new InternalServerKnownException("Request to retrieve latest destination definitions failed", e);
     }

diff --git a/airbyte-server/src/main/java/io/airbyte/server/services/AirbyteGithubStore.java b/airbyte-server/src/main/java/io/airbyte/server/services/AirbyteGithubStore.java
@@ -34,13 +34,18 @@
 import java.net.http.HttpRequest;
 import java.net.http.HttpResponse.BodyHandlers;
 import java.time.Duration;
+import java.util.Collections;
 import java.util.List;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Convenience class for retrieving files checked into the Airbyte Github repo.
  */
 public class AirbyteGithubStore {
 
+  private static final Logger LOGGER = LoggerFactory.getLogger(AirbyteGithubStore.class);
+
   private static final String GITHUB_BASE_URL = "https://raw.githubusercontent.com";
   private static final String SOURCE_DEFINITION_LIST_LOCATION_PATH =
       "/airbytehq/airbyte/master/airbyte-config/init/src/main/resources/seed/source_definitions.yaml";
@@ -65,12 +70,26 @@ public AirbyteGithubStore(String baseUrl, Duration timeout) {
     this.timeout = timeout;
   }
 
-  public List<StandardDestinationDefinition> getLatestDestinations() throws IOException, InterruptedException {
-    return YamlListToStandardDefinitions.toStandardDestinationDefinitions(getFile(DESTINATION_DEFINITION_LIST_LOCATION_PATH));
+  public List<StandardDestinationDefinition> getLatestDestinations() throws InterruptedException {
+    try {
+      return YamlListToStandardDefinitions.toStandardDestinationDefinitions(getFile(DESTINATION_DEFINITION_LIST_LOCATION_PATH));
+    } catch (IOException e) {
+      LOGGER.warn(
+          "Unable to retrieve latest Destination list from Github. Using the list bundled with Airbyte. This warning is expected if this Airbyte cluster does not have internet access.",
+          e);
+      return Collections.emptyList();
+    }
   }
 
-  public List<StandardSourceDefinition> getLatestSources() throws IOException, InterruptedException {
-    return YamlListToStandardDefinitions.toStandardSourceDefinitions(getFile(SOURCE_DEFINITION_LIST_LOCATION_PATH));
+  public List<StandardSourceDefinition> getLatestSources() throws InterruptedException {
+    try {
+      return YamlListToStandardDefinitions.toStandardSourceDefinitions(getFile(SOURCE_DEFINITION_LIST_LOCATION_PATH));
+    } catch (IOException e) {
+      LOGGER.warn(
+          "Unable to retrieve latest Source list from Github. Using the list bundled with Airbyte. This warning is expected if this Airbyte cluster does not have internet access.",
+          e);
+      return Collections.emptyList();
+    }
   }
 
   @VisibleForTesting
@@ -80,7 +99,11 @@ String getFile(String filePathWithSlashPrefix) throws IOException, InterruptedEx
         .timeout(timeout)
         .header("accept", "*/*") // accept any file type
         .build();
-    return httpClient.send(request, BodyHandlers.ofString()).body();
+    final var resp = httpClient.send(request, BodyHandlers.ofString());
+    if (resp.statusCode() >= 400) {
+      throw new IOException("getFile request ran into status code error: " + resp.statusCode() + "with message: " + resp.getClass());
+    }
+    return resp.body();
   }
 
 }
diff --git a/...te-server/src/test/java/io/airbyte/server/handlers/DestinationDefinitionsHandlerTest.java b/...te-server/src/test/java/io/airbyte/server/handlers/DestinationDefinitionsHandlerTest.java
@@ -192,7 +192,7 @@ class listLatest {
 
     @Test
     @DisplayName("should return the latest list")
-    void testCorrect() throws IOException, InterruptedException {
+    void testCorrect() throws InterruptedException {
       final StandardDestinationDefinition destinationDefinition = generateDestination();
       when(githubStore.getLatestDestinations()).thenReturn(Collections.singletonList(destinationDefinition));
 
@@ -205,8 +205,7 @@ void testCorrect() throws IOException, InterruptedException {
 
     @Test
     @DisplayName("returns empty collection if cannot find latest definitions")
-    void testHttpTimeout() throws IOException, InterruptedException {
-      when(githubStore.getLatestDestinations()).thenThrow(new IOException());
+    void testHttpTimeout() {
       assertEquals(0, destinationHandler.listLatestDestinationDefinitions().getDestinationDefinitions().size());
     }
 

diff --git a/airbyte-server/src/test/java/io/airbyte/server/handlers/SourceDefinitionsHandlerTest.java b/airbyte-server/src/test/java/io/airbyte/server/handlers/SourceDefinitionsHandlerTest.java
@@ -207,8 +207,7 @@ void testCorrect() throws IOException, InterruptedException {
 
     @Test
     @DisplayName("returns empty collection if cannot find latest definitions")
-    void testHttpTimeout() throws IOException, InterruptedException {
-      when(githubStore.getLatestSources()).thenThrow(new IOException());
+    void testHttpTimeout() {
       assertEquals(0, sourceHandler.listLatestSourceDefinitions().getSourceDefinitions().size());
     }
 

diff --git a/airbyte-server/src/test/java/io/airbyte/server/services/AirbyteGithubStoreTest.java b/airbyte-server/src/test/java/io/airbyte/server/services/AirbyteGithubStoreTest.java
@@ -30,10 +30,11 @@
 import java.io.IOException;
 import java.net.http.HttpTimeoutException;
 import java.time.Duration;
+import java.util.Collections;
 import java.util.concurrent.TimeUnit;
 import okhttp3.mockwebserver.MockResponse;
 import okhttp3.mockwebserver.MockWebServer;
-import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
@@ -45,15 +46,55 @@ public class AirbyteGithubStoreTest {
   private static MockWebServer webServer;
   private static AirbyteGithubStore githubStore;
 
-  @BeforeAll
-  public static void setUp() {
+  @BeforeEach
+  public void setUp() {
     webServer = new MockWebServer();
     githubStore = AirbyteGithubStore.test(webServer.url("/").toString(), TIMEOUT);
   }
 
+  @Nested
+  @DisplayName("when there is no internet")
+  class NoInternet {
+
+    @Test
+    void testGetLatestDestinations() throws InterruptedException, IOException {
+      webServer.shutdown();
+      assertEquals(Collections.emptyList(), githubStore.getLatestDestinations());
+    }
+
+    @Test
+    void testGetLatestSources() throws InterruptedException, IOException {
+      webServer.shutdown();
+      assertEquals(Collections.emptyList(), githubStore.getLatestSources());
+    }
+
+  }
+
+  @Nested
+  @DisplayName("when a bad file is specified")
+  class BadFile {
+
+    @Test
+    void testGetLatestDestinations() throws InterruptedException {
+      final var timeoutResp = new MockResponse().setResponseCode(404);
+      webServer.enqueue(timeoutResp);
+
+      assertEquals(Collections.emptyList(), githubStore.getLatestDestinations());
+    }
+
+    @Test
+    void testGetLatestSources() throws InterruptedException {
+      final var timeoutResp = new MockResponse().setResponseCode(404);
+      webServer.enqueue(timeoutResp);
+
+      assertEquals(Collections.emptyList(), githubStore.getLatestSources());
+    }
+
+  }
+
   @Nested
   @DisplayName("getFile")
-  class getFile {
+  class GetFile {
 
     @Test
     void testReturn() throws IOException, InterruptedException {