config-loader: apply review feedback

Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>

packages/config-loader/api-report.md

@@ -5,6 +5,7 @@
 ```ts
 import { AppConfig } from '@backstage/config';
 import { Config } from '@backstage/config';
+import { HumanDuration } from '@backstage/types';
 import { JsonObject } from '@backstage/types';
 import { JSONSchema7 } from 'json-schema';
 import { Observable } from '@backstage/types';
@@ -31,7 +32,7 @@ export interface AsyncConfigSourceIterator
 // @public
 export interface BaseConfigSourcesOptions {
   // (undocumented)
-  remote?: Pick<RemoteConfigSourceOptions, 'reloadIntervalSeconds'>;
+  remote?: Pick<RemoteConfigSourceOptions, 'reloadInterval'>;
   // (undocumented)
   rootDir?: string;
   // (undocumented)
@@ -246,7 +247,7 @@ export class RemoteConfigSource implements ConfigSource {
 
 // @public
 export interface RemoteConfigSourceOptions {
-  reloadIntervalSeconds?: number;
+  reloadInterval?: HumanDuration;
   substitutionFunc?: EnvFunc;
   url: string;
 }

packages/config-loader/package.json

@@ -44,8 +44,8 @@
     "json-schema": "^0.4.0",
     "json-schema-merge-allof": "^0.8.1",
     "json-schema-traverse": "^1.0.0",
-    "lodash": "^4.14.151",
-    "minimist": "^1.2.8",
+    "lodash": "^4.17.21",
+    "minimist": "^1.2.5",
     "node-fetch": "^2.6.7",
     "typescript-json-schema": "^0.55.0",
     "yaml": "^2.0.0",

packages/config-loader/src/loader.ts

@@ -104,7 +104,9 @@ export async function loadConfig(
 ): Promise<LoadConfigResult> {
   const source = ConfigSources.default({
     substitutionFunc: options.experimentalEnvFunc,
-    remote: options.remote,
+    remote: options.remote && {
+      reloadInterval: { seconds: options.remote.reloadIntervalSeconds },
+    },
     rootDir: options.configRoot,
     argv: options.configTargets.flatMap(t => [
       '--config',

packages/config-loader/src/sources/ConfigSources.test.ts

@@ -135,14 +135,14 @@ describe('ConfigSources', () => {
         ConfigSources.defaultForTargets({
           rootDir: '/',
           targets: [{ type: 'url', target: 'http://example.com/config.yaml' }],
-          remote: { reloadIntervalSeconds: 5 },
+          remote: { reloadInterval: { minutes: 2 } },
         }),
       ),
     ).toEqual([
       {
         name: 'RemoteConfigSource',
         url: 'http://example.com/config.yaml',
-        reloadIntervalSeconds: 5,
+        reloadInterval: { minutes: 2 },
       },
     ]);
   });

packages/config-loader/src/sources/ConfigSources.ts

@@ -72,7 +72,7 @@ export interface ClosableConfig extends Config {
  */
 export interface BaseConfigSourcesOptions {
   rootDir?: string;
-  remote?: Pick<RemoteConfigSourceOptions, 'reloadIntervalSeconds'>;
+  remote?: Pick<RemoteConfigSourceOptions, 'reloadInterval'>;
   substitutionFunc?: SubstitutionFunc;
 }
 
@@ -112,8 +112,12 @@ export class ConfigSources {
     const args: string[] = [parseArgs(argv).config].flat().filter(Boolean);
     return args.map(target => {
       try {
-        // eslint-disable-next-line no-new
-        new URL(target);
+        const url = new URL(target);
+
+        // Some file paths are valid relative URLs, so check if the host is empty too
+        if (!url.host) {
+          return { type: 'path', target };
+        }
         return { type: 'url', target };
       } catch {
         return { type: 'path', target };
@@ -151,7 +155,7 @@ export class ConfigSources {
         return RemoteConfigSource.create({
           url: arg.target,
           substitutionFunc: options.substitutionFunc,
-          reloadIntervalSeconds: options.remote.reloadIntervalSeconds,
+          reloadInterval: options.remote.reloadInterval,
         });
       }
       return FileConfigSource.create({

packages/config-loader/src/sources/FileConfigSource.ts

@@ -147,9 +147,11 @@ export class FileConfigSource implements ConfigSource {
       }
     };
 
-    signal?.addEventListener('abort', () => {
+    const onAbort = () => {
+      signal?.removeEventListener('abort', onAbort);
       watcher.close();
-    });
+    };
+    signal?.addEventListener('abort', onAbort);
 
     yield { configs: await readConfigFile() };
 

packages/config-loader/src/sources/RemoteConfigSource.test.ts

@@ -74,7 +74,7 @@ app:
 
     const source = RemoteConfigSource.create({
       url: 'http://localhost/config.yaml',
-      reloadIntervalSeconds: 0,
+      reloadInterval: { seconds: 0 },
     });
 
     await expect(readN(source, 2)).resolves.toEqual([

packages/config-loader/src/sources/RemoteConfigSource.ts

@@ -15,7 +15,7 @@
  */
 
 import { ResponseError } from '@backstage/errors';
-import { JsonObject } from '@backstage/types';
+import { HumanDuration, JsonObject } from '@backstage/types';
 import isEqual from 'lodash/isEqual';
 import fetch from 'node-fetch';
 import yaml from 'yaml';
@@ -27,7 +27,28 @@ import {
   ReadConfigDataOptions,
 } from './types';
 
-const DEFAULT_RELOAD_INTERVAL_SECONDS = 60;
+const DEFAULT_RELOAD_INTERVAL = { seconds: 60 };
+
+function durationToMs(duration: HumanDuration): number {
+  const {
+    years = 0,
+    months = 0,
+    weeks = 0,
+    days = 0,
+    hours = 0,
+    minutes = 0,
+    seconds = 0,
+    milliseconds = 0,
+  } = duration;
+
+  const totalDays = years * 365 + months * 30 + weeks * 7 + days;
+  const totalHours = totalDays * 24 + hours;
+  const totalMinutes = totalHours * 60 + minutes;
+  const totalSeconds = totalMinutes * 60 + seconds;
+  const totalMilliseconds = totalSeconds * 1000 + milliseconds;
+
+  return totalMilliseconds;
+}
 
 /**
  * Options for {@link RemoteConfigSource.create}.
@@ -45,7 +66,7 @@ export interface RemoteConfigSourceOptions {
    *
    * Set to Infinity to disable reloading.
    */
-  reloadIntervalSeconds?: number;
+  reloadInterval?: HumanDuration;
 
   /**
    * A substitution function to use instead of the default environment substitution.
@@ -78,13 +99,14 @@ export class RemoteConfigSource implements ConfigSource {
   }
 
   readonly #url: string;
-  readonly #reloadIntervalSeconds: number;
+  readonly #reloadIntervalMs: number;
   readonly #transformer: ConfigTransformer;
 
   private constructor(options: RemoteConfigSourceOptions) {
     this.#url = options.url;
-    this.#reloadIntervalSeconds =
-      options.reloadIntervalSeconds ?? DEFAULT_RELOAD_INTERVAL_SECONDS;
+    this.#reloadIntervalMs = durationToMs(
+      options.reloadInterval ?? DEFAULT_RELOAD_INTERVAL,
+    );
     this.#transformer = createConfigTransformer({
       substitutionFunc: options.substitutionFunc,
     });
@@ -98,12 +120,12 @@ export class RemoteConfigSource implements ConfigSource {
     yield { configs: [{ data, context: this.#url }] };
 
     for (;;) {
+      await this.#wait(options?.signal);
+
       if (options?.signal?.aborted) {
         return;
       }
 
-      await this.#wait(options?.signal);
-
       try {
         const newData = await this.#load(options?.signal);
         if (newData && !isEqual(data, newData)) {
@@ -146,7 +168,7 @@ export class RemoteConfigSource implements ConfigSource {
 
   async #wait(signal?: AbortSignal) {
     return new Promise<void>(resolve => {
-      const timeoutId = setTimeout(onDone, this.#reloadIntervalSeconds * 1000);
+      const timeoutId = setTimeout(onDone, this.#reloadIntervalMs);
       signal?.addEventListener('abort', onDone);
 
       function onDone() {