From d67437149521027c9f5ce5785c2929af0f9afb7a Mon Sep 17 00:00:00 2001 From: blam Date: Thu, 14 Nov 2024 09:20:05 -0700 Subject: [PATCH 1/6] chore: adjusting the types of the checkpoint interface slightly Signed-off-by: blam --- .../tasks/NunjucksWorkflowRunner.test.ts | 34 +++++++++++++++---- .../tasks/NunjucksWorkflowRunner.ts | 23 ++++++++----- plugins/scaffolder-node/src/actions/types.ts | 8 ++--- 3 files changed, 47 insertions(+), 18 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts index 25caf7bad0..b76356a586 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts @@ -152,19 +152,41 @@ describe('NunjucksWorkflowRunner', () => { id: 'checkpoints-action', description: 'Mock action with checkpoints', handler: async ctx => { - const key1 = await ctx.checkpoint('key1', async () => { - return 'updated'; + const key1 = await ctx.checkpoint({ + key: 'key1', + fn: async () => { + return 'updated'; + }, }); - const key2 = await ctx.checkpoint('key2', async () => { - return 'updated'; + const key2 = await ctx.checkpoint({ + key: 'key2', + fn: async () => { + return 'updated'; + }, }); - const key3 = await ctx.checkpoint('key3', async () => { - return 'updated'; + const key3 = await ctx.checkpoint({ + key: 'key3', + fn: async () => { + return 'updated'; + }, + }); + + const key4 = await ctx.checkpoint({ + fn: () => {}, + }); + + const key5 = await ctx.checkpoint({ + fn: async () => {}, }); ctx.output('key1', key1); ctx.output('key2', key2); ctx.output('key3', key3); + + // @ts-expect-error - this is void return + ctx.output('key4', key4); + // @ts-expect-error - this is void return + ctx.output('key5', key5); }, }); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index 6f7551568f..8a4cb8767f 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -364,6 +364,8 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { )}`, ); } + let checkpointCount = 0; + await action.handler({ input: iteration.input, secrets: task.secrets ?? {}, @@ -371,21 +373,26 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { logger: loggerToWinstonLogger(taskLogger), logStream: streamLogger, workspacePath, - async checkpoint( - keySuffix: string, - fn: () => Promise, - ) { - const key = `v1.task.checkpoint.${step.id}.${keySuffix}`; + async checkpoint(opts: { + key?: string; + fn: () => Promise | T; + }) { + const { key: checkpointKey = checkpointCount++, fn } = opts; + // default the task checkpoint to stepId plus count if there's no explict key provided. + const key = `v1.task.checkpoint.${step.id}.${checkpointKey}`; + try { - let prevValue: U | undefined; + let prevValue: T | undefined; + if (prevTaskState) { const prevState = ( prevTaskState.state?.checkpoints as { [key: string]: CheckpointState; } )?.[key]; + if (prevState && prevState.status === 'success') { - prevValue = prevState.value as U; + prevValue = prevState.value as T; } } @@ -395,7 +402,7 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { task.updateCheckpoint?.({ key, status: 'success', - value, + value: value ?? {}, }); } return value; diff --git a/plugins/scaffolder-node/src/actions/types.ts b/plugins/scaffolder-node/src/actions/types.ts index f0e4c12869..f24dc154e2 100644 --- a/plugins/scaffolder-node/src/actions/types.ts +++ b/plugins/scaffolder-node/src/actions/types.ts @@ -38,10 +38,10 @@ export type ActionContext< secrets?: TaskSecrets; workspacePath: string; input: TActionInput; - checkpoint( - key: string, - fn: () => Promise, - ): Promise; + checkpoint(opts: { + key?: string; + fn: () => Promise | U; + }): Promise; output( name: keyof TActionOutput, value: TActionOutput[keyof TActionOutput], From 93aa2fb4d1174d232b9411f3c89355f6c35b5a06 Mon Sep 17 00:00:00 2001 From: blam Date: Thu, 14 Nov 2024 09:28:46 -0700 Subject: [PATCH 2/6] chore: update more tests Signed-off-by: blam --- .../tasks/NunjucksWorkflowRunner.test.ts | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts index b76356a586..24ec456223 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts @@ -154,21 +154,15 @@ describe('NunjucksWorkflowRunner', () => { handler: async ctx => { const key1 = await ctx.checkpoint({ key: 'key1', - fn: async () => { - return 'updated'; - }, + fn: async () => 'updated', }); const key2 = await ctx.checkpoint({ key: 'key2', - fn: async () => { - return 'updated'; - }, + fn: async () => 'updated', }); const key3 = await ctx.checkpoint({ key: 'key3', - fn: async () => { - return 'updated'; - }, + fn: async () => 'updated', }); const key4 = await ctx.checkpoint({ @@ -179,6 +173,10 @@ describe('NunjucksWorkflowRunner', () => { fn: async () => {}, }); + const key6 = await ctx.checkpoint({ + fn: async () => 'look ma no key', + }); + ctx.output('key1', key1); ctx.output('key2', key2); ctx.output('key3', key3); @@ -187,6 +185,8 @@ describe('NunjucksWorkflowRunner', () => { ctx.output('key4', key4); // @ts-expect-error - this is void return ctx.output('key5', key5); + + ctx.output('key6', key6); }, }); @@ -654,6 +654,9 @@ describe('NunjucksWorkflowRunner', () => { key1: '${{steps.test.output.key1}}', key2: '${{steps.test.output.key2}}', key3: '${{steps.test.output.key3}}', + key4: '${{steps.test.output.key4}}', + key5: '${{steps.test.output.key5}}', + key6: '${{steps.test.output.key6}}', }, }), getTaskState: (): Promise< @@ -683,6 +686,9 @@ describe('NunjucksWorkflowRunner', () => { expect(result.output.key1).toEqual('initial'); expect(result.output.key2).toEqual('updated'); expect(result.output.key3).toEqual('updated'); + expect(result.output.key4).toEqual(undefined); + expect(result.output.key5).toEqual(undefined); + expect(result.output.key6).toEqual('look ma no key'); }); it('should template the output from simple actions', async () => { From e61d5ef286c077092ecb14f25b324c7df2065a6e Mon Sep 17 00:00:00 2001 From: blam Date: Thu, 14 Nov 2024 09:34:41 -0700 Subject: [PATCH 3/6] chore: changeset Signed-off-by: blam --- .changeset/wild-horses-refuse.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 .changeset/wild-horses-refuse.md diff --git a/.changeset/wild-horses-refuse.md b/.changeset/wild-horses-refuse.md new file mode 100644 index 0000000000..6d0657273c --- /dev/null +++ b/.changeset/wild-horses-refuse.md @@ -0,0 +1,12 @@ +--- +'@backstage/plugin-scaffolder-backend': minor +'@backstage/plugin-scaffolder-node': minor +--- + +BREAKING ALPHA: The `checkpoint` method now takes an object instead of previous arguments, which allows also for an optional `key` which will be an incrementing counter for each call of the `checkpoint` method per step. + +```ts +await ctx.checkpoint({ key: 'repo.create', fn: () => ockokit.repo.create({...})}) +``` + +You can also now return `void` from the checkpoint if the method returns `void` inside the `checkpoint` handler. From 5483d43b0d449444d870426ae232cd8758363ceb Mon Sep 17 00:00:00 2001 From: blam Date: Thu, 14 Nov 2024 10:01:45 -0700 Subject: [PATCH 4/6] chore: api-reports + revert optional key change Signed-off-by: blam --- .changeset/wild-horses-refuse.md | 2 +- .../src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts | 9 ++------- .../src/scaffolder/tasks/NunjucksWorkflowRunner.ts | 4 +--- plugins/scaffolder-node/report.api.md | 8 ++++---- plugins/scaffolder-node/src/actions/types.ts | 2 +- 5 files changed, 9 insertions(+), 16 deletions(-) diff --git a/.changeset/wild-horses-refuse.md b/.changeset/wild-horses-refuse.md index 6d0657273c..bba991b057 100644 --- a/.changeset/wild-horses-refuse.md +++ b/.changeset/wild-horses-refuse.md @@ -3,7 +3,7 @@ '@backstage/plugin-scaffolder-node': minor --- -BREAKING ALPHA: The `checkpoint` method now takes an object instead of previous arguments, which allows also for an optional `key` which will be an incrementing counter for each call of the `checkpoint` method per step. +BREAKING ALPHA: The `checkpoint` method now takes an object instead of previous arguments. ```ts await ctx.checkpoint({ key: 'repo.create', fn: () => ockokit.repo.create({...})}) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts index 24ec456223..9bef42b26a 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts @@ -166,17 +166,15 @@ describe('NunjucksWorkflowRunner', () => { }); const key4 = await ctx.checkpoint({ + key: 'key4', fn: () => {}, }); const key5 = await ctx.checkpoint({ + key: 'key5', fn: async () => {}, }); - const key6 = await ctx.checkpoint({ - fn: async () => 'look ma no key', - }); - ctx.output('key1', key1); ctx.output('key2', key2); ctx.output('key3', key3); @@ -185,8 +183,6 @@ describe('NunjucksWorkflowRunner', () => { ctx.output('key4', key4); // @ts-expect-error - this is void return ctx.output('key5', key5); - - ctx.output('key6', key6); }, }); @@ -688,7 +684,6 @@ describe('NunjucksWorkflowRunner', () => { expect(result.output.key3).toEqual('updated'); expect(result.output.key4).toEqual(undefined); expect(result.output.key5).toEqual(undefined); - expect(result.output.key6).toEqual('look ma no key'); }); it('should template the output from simple actions', async () => { diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index 8a4cb8767f..c5413a08e0 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -364,7 +364,6 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { )}`, ); } - let checkpointCount = 0; await action.handler({ input: iteration.input, @@ -377,8 +376,7 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { key?: string; fn: () => Promise | T; }) { - const { key: checkpointKey = checkpointCount++, fn } = opts; - // default the task checkpoint to stepId plus count if there's no explict key provided. + const { key: checkpointKey, fn } = opts; const key = `v1.task.checkpoint.${step.id}.${checkpointKey}`; try { diff --git a/plugins/scaffolder-node/report.api.md b/plugins/scaffolder-node/report.api.md index 6328f94bfd..c7552a2eb6 100644 --- a/plugins/scaffolder-node/report.api.md +++ b/plugins/scaffolder-node/report.api.md @@ -31,10 +31,10 @@ export type ActionContext< secrets?: TaskSecrets; workspacePath: string; input: TActionInput; - checkpoint( - key: string, - fn: () => Promise, - ): Promise; + checkpoint(opts: { + key: string; + fn: () => Promise | U; + }): Promise; output( name: keyof TActionOutput, value: TActionOutput[keyof TActionOutput], diff --git a/plugins/scaffolder-node/src/actions/types.ts b/plugins/scaffolder-node/src/actions/types.ts index f24dc154e2..90a8a741b0 100644 --- a/plugins/scaffolder-node/src/actions/types.ts +++ b/plugins/scaffolder-node/src/actions/types.ts @@ -39,7 +39,7 @@ export type ActionContext< workspacePath: string; input: TActionInput; checkpoint(opts: { - key?: string; + key: string; fn: () => Promise | U; }): Promise; output( From b402097db32750d5028bae2cad00cbe0e0b48f5a Mon Sep 17 00:00:00 2001 From: blam Date: Thu, 14 Nov 2024 10:06:30 -0700 Subject: [PATCH 5/6] chore: rename type parameter as it's not a union Signed-off-by: blam --- plugins/scaffolder-node/report.api.md | 6 +++--- plugins/scaffolder-node/src/actions/types.ts | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/plugins/scaffolder-node/report.api.md b/plugins/scaffolder-node/report.api.md index c7552a2eb6..c14a689d1a 100644 --- a/plugins/scaffolder-node/report.api.md +++ b/plugins/scaffolder-node/report.api.md @@ -31,10 +31,10 @@ export type ActionContext< secrets?: TaskSecrets; workspacePath: string; input: TActionInput; - checkpoint(opts: { + checkpoint(opts: { key: string; - fn: () => Promise | U; - }): Promise; + fn: () => Promise | T; + }): Promise; output( name: keyof TActionOutput, value: TActionOutput[keyof TActionOutput], diff --git a/plugins/scaffolder-node/src/actions/types.ts b/plugins/scaffolder-node/src/actions/types.ts index 90a8a741b0..5dd9d5526f 100644 --- a/plugins/scaffolder-node/src/actions/types.ts +++ b/plugins/scaffolder-node/src/actions/types.ts @@ -38,10 +38,10 @@ export type ActionContext< secrets?: TaskSecrets; workspacePath: string; input: TActionInput; - checkpoint(opts: { + checkpoint(opts: { key: string; - fn: () => Promise | U; - }): Promise; + fn: () => Promise | T; + }): Promise; output( name: keyof TActionOutput, value: TActionOutput[keyof TActionOutput], From 12be0b3631c1aa6dc0e684932da1e47e489df815 Mon Sep 17 00:00:00 2001 From: blam Date: Thu, 14 Nov 2024 10:07:53 -0700 Subject: [PATCH 6/6] chore: updating changeset to be experimental breaking Signed-off-by: blam --- .changeset/wild-horses-refuse.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/wild-horses-refuse.md b/.changeset/wild-horses-refuse.md index bba991b057..9dc0c5a15d 100644 --- a/.changeset/wild-horses-refuse.md +++ b/.changeset/wild-horses-refuse.md @@ -3,7 +3,7 @@ '@backstage/plugin-scaffolder-node': minor --- -BREAKING ALPHA: The `checkpoint` method now takes an object instead of previous arguments. +BREAKING EXPERIMENTAL: The `checkpoint` method now takes an object instead of previous arguments. ```ts await ctx.checkpoint({ key: 'repo.create', fn: () => ockokit.repo.create({...})})