refactor!: Create cron-node (MAPCO-10228) by Powfu2 · Pull Request #41 · MapColonies/planet-dumper

Powfu2 · 2026-07-19T08:32:36Z

Create cron-node,
upgrade all libraries,
add ci
add tests

github-actions · 2026-07-19T08:32:46Z

🎫 Related Jira Issue: MAPCO-10228

eyalr1100 · 2026-07-19T11:14:06Z

 {
  "telemetry": {
+    "shared": {
+      "serviceName": "planet-dumper"


do shared: {}

eyalr1100 · 2026-07-19T11:18:39Z

+  {{- if .Values.cron.enabled }}
+  TARGET: {{ .Values.cron.target | quote }}
+  CRON_EXPRESSION: {{ .Values.cron.schedule | quote }}
+  RUN_ON_INIT: {{ .Values.cron.runOnInit | quote }}
+  {{- end }}


I like with .values.cron but that's me

eyalr1100 · 2026-07-19T11:33:11Z

+  try {
+    const state = await manager.getState(stateSource);
+
+    // pre cleanup


remove ai comments

eyalr1100 · 2026-07-19T11:34:56Z

+    if (cleanupMode === 'post-clean-workdir') {
+      await emptyDirectory(join(WORKDIR, state));
+    } else if (cleanupMode === 'post-clean-others') {
+      await emptyDirectory(WORKDIR, [state]);
+    } else if (cleanupMode === 'post-clean-all') {
+      await emptyDirectory(WORKDIR);


UGLY! maybe switch? Maybe enum (as const, satisfy, whatever)?

eyalr1100 · 2026-07-19T11:37:56Z

+export const PG_DUMP_CLEANUP_CHOICES: CleanupMode[] = ['none', 'pre-clean-others', 'post-clean-others'];
+
+export const CREATE_CLEANUP_CHOICES: ExtendedCleanupMode[] = [
+  'none',
+  'pre-clean-others',
+  'post-clean-others',
+  'post-clean-workdir',
+  'post-clean-all',
+];
+


AN ARRAY??? WITH LOW CARDINALITY VALUES???

eyalr1100 · 2026-07-19T12:54:45Z

+const buildArgv = (overrides: Partial<ScheduleArguments>): Arguments<ScheduleArguments> => ({
+  // eslint-disable-next-line @typescript-eslint/naming-convention -- required by yargs' Arguments<T> shape
+  _: [],
+  $0: 'planet-dumper',
+  outputFormat: 'dump_{state}_{timestamp}.pbf',
+  stateSource: '1',
+  cleanupMode: 'none',
+  s3Acl: 'private',
+  dumpServerHeaders: [],
+  resume: false,
+  info: false,
+  target: 'pg_dump',
+  cronExpression: '* * * * * *',
+  runOnInit: false,
+  ...overrides,
+});


I feel like this could be in a different class

eyalr1100 · 2026-07-19T12:55:05Z

+    for (const task of getTasks().values()) {
+      void task.destroy();
+    }
+    vi.restoreAllMocks();
+  });


eyalr1100 · 2026-07-19T12:55:56Z

+    // gate the first run on an explicit signal instead of a fixed delay, so "still in-flight"
+    // is guaranteed rather than timing-dependent on how fast the test happens to run


eyalr1100 · 2026-07-19T13:00:14Z

+const disabledArstotzkaConfig: ArstotzkaConfig = {
+  enabled: false,
+  services: { planetDumperPg: 'pg-service', planetDumperNg: 'ng-service' },
+  mediator: {
+    timeout: 1000,
+    enableRetryStrategy: false,
+    retryStrategy: {},
+  },
+};
+
+const enabledArstotzkaConfig: ArstotzkaConfig = {
+  enabled: true,
+  services: { planetDumperPg: 'pg-service', planetDumperNg: 'ng-service' },
+  mediator: {
+    timeout: 1000,
+    enableRetryStrategy: false,
+    retryStrategy: {},
+    actiony: { url: 'http://actiony.example.com' },
+    locky: { url: 'http://locky.example.com' },
+  },
+};


instead of writing the same cofnig twice with one variable flipped, just use ... with overrides

eyalr1100 · 2026-07-19T13:02:40Z

@@ -0,0 +1,3 @@
+{
+  ".": "6.1.0"


The master tag says 4.0.0

eyalr1100 · 2026-07-22T08:09:27Z

+      let exitCode = ExitCodes.GENERAL_ERROR;
+
+      if (error instanceof ErrorWithExitCode) {
+        exitCode = error.exitCode;
+      }
+


Suggested change

let exitCode = ExitCodes.GENERAL_ERROR;

if (error instanceof ErrorWithExitCode) {

exitCode = error.exitCode;

}

const exitCode = error instanceof ErrorWithExitCode ? error.exitCode : ExitCodes.GENERAL_ERROR;

eyalr1100 · 2026-07-22T08:22:27Z

+}): DependencyContainer => {
+  const logger = overrides.logger ?? buildLogger();
+  const config = overrides.config ?? buildConfig();
+  const manager = overrides.manager ?? ({} as CreateManager);


eyalr1100 · 2026-07-22T08:27:32Z

+    "@map-colonies/config": "^4.0.1",
+    "@map-colonies/js-logger": "^5.0.0",
+    "@map-colonies/read-pkg": "^2.0.0",
+    "@map-colonies/schemas": "https://ghatmpstorage.blob.core.windows.net/npm-packages/schemas-213f62397c5cf2f0f83694fbcdaf18665b00bd6e.tgz",


don't forget to change

eyalr1100 · 2026-08-12T15:50:49Z

You didn't fix this file at all

eyalr1100 · 2026-08-13T07:46:18Z

+const runCreatePipelineMock = vi.mocked(runCreatePipeline);
+const terminateChildrenMock = vi.mocked(terminateChildren);
+
+const buildLogger = (): Logger => ({ debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }) as unknown as Logger;


fix this, as unknown as is a bad habbit, I'd rather you create a vi.fn() for every logger function instead
Also I don't think you need to test the logs so you can remove logger altogether

eyalr1100 · 2026-08-13T08:04:34Z

+
+const spawnChildMock = vi.mocked(spawnChild);
+
+const buildAxios = (): AxiosInstance => ({ get: vi.fn() }) as unknown as AxiosInstance;


NivGreenstein · 2026-08-18T09:25:16Z

Any breaking changes? if so please rename to refactor!:

NivGreenstein · 2026-08-18T09:32:58Z

-            - configMapRef:
-                name: {{ $releaseName }}-{{ $chartName }}-command-options-configmap


NivGreenstein · 2026-08-18T09:55:04Z

  enabled: false
-  apiVersion: 'batch/v1' # depends on openshift version batch/v1 or batch/v1beta1
-  restartPolicy: OnFailure
  schedule: '* * * * *'


is this the actual schedule??

NivGreenstein · 2026-08-19T05:41:21Z

-      { token: SERVICES.ARSTOTZKA, provider: { useValue: arstotzkaConfig } },
-      { token: SERVICES.HTTP_CLIENT, provider: { useValue: axiosClient } },
+      { token: SCHEDULE_COMMAND_FACTORY, provider: { useFactory: scheduleCommandFactory } },
+      { token: SERVICES.CONFIG, provider: { useFactory: () => getConfig() } },


change to useValue

NivGreenstein · 2026-08-19T05:44:23Z

+            if (endpoint === undefined) {
+              throw new CheckError('s3.endpoint must be configured to use the s3 client', 's3', { endpoint });
+            }
+            return new S3Client({ endpoint, region: S3_REGION, forcePathStyle: true });
+          }),


create a factory function somewhere else + utilize the @map-colonies/schemas validation.

NivGreenstein · 2026-08-19T05:54:58Z

 {
  "name": "planet-dumper",
-  "version": "6.1.0",
+  "version": "4.0.0",


revert version

NivGreenstein · 2026-08-19T05:57:39Z

-    "tsyringe": "^4.6.0",
+    "tsyringe": "^4.10.0",
    "valid-url": "^1.0.9",
    "yargs": "^17.1.0"


NivGreenstein · 2026-08-19T05:57:50Z

+    "@map-colonies/config": "^4.0.1",
+    "@map-colonies/js-logger": "^5.0.0",
+    "@map-colonies/read-pkg": "^2.0.0",
+    "@map-colonies/schemas": "https://ghatmpstorage.blob.core.windows.net/npm-packages/schemas-120a0c5e573e9bc4c79b7e346950f4eb5835add9.tgz",


don't forger to change

Powfu2 added 2 commits July 19, 2026 10:53

refactor: add-node-cron

0188bf5

ci: add ci

dc0dcac

Powfu2 requested a review from eyalr1100 July 19, 2026 08:32

eyalr1100 reviewed Jul 19, 2026

View reviewed changes

fix: add tests and create fs repo

d5102a6

eyalr1100 self-requested a review July 22, 2026 06:32

eyalr1100 reviewed Jul 22, 2026

View reviewed changes

fix: update tests

2a9562e

eyalr1100 requested changes Aug 13, 2026

View reviewed changes

fix: upgrade-tests

260d048

Powfu2 requested review from NivGreenstein and eyalr1100 August 17, 2026 10:30

NivGreenstein requested changes Aug 19, 2026

View reviewed changes

Powfu2 changed the title ~~refactor: Create cron-node (MAPCO-10228)~~ refactor!: Create cron-node (MAPCO-10228) Aug 19, 2026

fix: add few logs

cea93a3

		// gate the first run on an explicit signal instead of a fixed delay, so "still in-flight"
		// is guaranteed rather than timing-dependent on how fast the test happens to run


		const spawnChildMock = vi.mocked(spawnChild);

		const buildAxios = (): AxiosInstance => ({ get: vi.fn() }) as unknown as AxiosInstance;

		- configMapRef:
		name: {{ $releaseName }}-{{ $chartName }}-command-options-configmap

Uh oh!

Conversation

Powfu2 commented Jul 19, 2026

Uh oh!

github-actions Bot commented Jul 19, 2026 • edited by atlassian Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NivGreenstein commented Aug 18, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

github-actions Bot commented Jul 19, 2026 •

edited by atlassian Bot

Loading