No retry option should disable retry policy (#1064)

allegro · Apr 24, 2023 · 97e1fef · 97e1fef
1 parent 61f6792
commit 97e1fef
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 5 deletions.
diff --git a/src/common/models/cluster/cluster.mocha.ts b/src/common/models/cluster/cluster.mocha.ts
@@ -18,7 +18,7 @@
 import { expect, use } from "chai";
 import equivalent from "../../../client/utils/test-utils/equivalent";
 import { RequestDecorator } from "../../../server/utils/request-decorator/request-decorator";
-import { RetryOptions } from "../../../server/utils/retry-options/retry-options";
+import { RetryOptions, RetryOptionsJS } from "../../../server/utils/retry-options/retry-options";
 import { NOOP_LOGGER } from "../../logger/logger";
 import { ClusterAuthJS } from "../cluster-auth/cluster-auth";
 import { Cluster, ClusterJS, fromConfig } from "./cluster";
@@ -48,7 +48,7 @@ describe("Cluster", () => {
         healthCheckTimeout: 1000,
         introspectionStrategy: "segment-metadata-fallback",
         requestDecorator: null,
-        retry: new RetryOptions(),
+        retry: undefined,
         sourceListRefreshInterval: 0,
         sourceListRefreshOnLoad: false,
         sourceListScan: "auto",
@@ -119,6 +119,16 @@ describe("Cluster", () => {
       expect(cluster.retry).to.be.equivalent(new RetryOptions({ maxAttempts: 1, delay: 42 }));
     });
 
+    it("should read partial retry options", () => {
+      const cluster = buildCluster({
+        retry: {
+          delay: 42
+        } as RetryOptionsJS
+      });
+
+      expect(cluster.retry).to.be.equivalent(new RetryOptions({ maxAttempts: 5, delay: 42 }));
+    });
+
     it("should read request decorator", () => {
       const cluster = buildCluster({
         name: "foobar",
@@ -185,7 +195,7 @@ describe("Cluster", () => {
         version: "new-version",
         type: "druid",
         requestDecorator: null,
-        retry: new RetryOptions(),
+        retry: undefined,
         auth: undefined
       });
     });

diff --git a/src/common/models/cluster/cluster.ts b/src/common/models/cluster/cluster.ts
@@ -127,6 +127,11 @@ function readRequestDecorator(cluster: any, logger: Logger): RequestDecorator |
   return null;
 }
 
+function readRetryOptions(options: RetryOptionsJS | undefined): RetryOptions | undefined {
+  if (isNil(options)) return undefined;
+  return new RetryOptions(options);
+}
+
 const DEFAULT_HEALTH_CHECK_TIMEOUT = 1000;
 export const DEFAULT_SOURCE_LIST_SCAN: SourceListScan = "auto";
 const SOURCE_LIST_SCAN_VALUES: SourceListScan[] = ["disable", "auto"];
@@ -167,7 +172,7 @@ export function fromConfig(params: ClusterJS, logger: Logger): Cluster {
   const sourceReintrospectInterval = readInterval(params.sourceReintrospectInterval, DEFAULT_SOURCE_REINTROSPECT_INTERVAL);
   const sourceListRefreshInterval = readInterval(params.sourceListRefreshInterval, DEFAULT_SOURCE_LIST_REFRESH_INTERVAL);
   const sourceTimeBoundaryRefreshInterval = readInterval(params.sourceTimeBoundaryRefreshInterval, DEFAULT_SOURCE_TIME_BOUNDARY_REFRESH_INTERVAL);
-  const retry = RetryOptions.fromJS(params.retry);
+  const retry = readRetryOptions(params.retry);
   const requestDecorator = readRequestDecorator(params, logger);
   const auth = readClusterAuth(params.auth);
 

diff --git a/src/server/utils/requester/requester.ts b/src/server/utils/requester/requester.ts
@@ -67,7 +67,8 @@ function setRetryOptions({ maxAttempts, delay }: RetryOptions) {
     requester,
     retry: maxAttempts,
     delay,
-    retryOnTimeout: true });
+    retryOnTimeout: true
+  });
 }
 
 function setVerbose(requester: PlywoodRequester<any>) {