From 7a85894b52e07c60587e3009de5192536b417756 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Thu, 17 Oct 2024 17:34:04 +0100 Subject: [PATCH 01/52] compiler: support multiple adaptors when handling imports --- .changeset/sharp-tips-pretend.md | 5 + .../compiler/src/transforms/add-imports.ts | 55 +++---- .../test/transforms/add-imports.test.ts | 139 +++++++++++++----- 3 files changed, 134 insertions(+), 65 deletions(-) create mode 100644 .changeset/sharp-tips-pretend.md diff --git a/.changeset/sharp-tips-pretend.md b/.changeset/sharp-tips-pretend.md new file mode 100644 index 000000000..bf36342bd --- /dev/null +++ b/.changeset/sharp-tips-pretend.md @@ -0,0 +1,5 @@ +--- +'@openfn/compiler': minor +--- + +support multiple adaptors when adding imports diff --git a/packages/compiler/src/transforms/add-imports.ts b/packages/compiler/src/transforms/add-imports.ts index d6c9ae36c..4828388ec 100644 --- a/packages/compiler/src/transforms/add-imports.ts +++ b/packages/compiler/src/transforms/add-imports.ts @@ -94,11 +94,11 @@ const globalRE = new RegExp(`^${globals.join('|')}$`); export type AddImportsOptions = { ignore?: string[]; // Adaptor MUST be pre-populated for this transformer to actually do anything - adaptor: { + adaptors: Array<{ name: string; exports?: string[]; exportAll?: boolean; - }; + }>; }; export type IdentifierList = Record; @@ -157,31 +157,34 @@ export function findAllDanglingIdentifiers(ast: ASTNode) { } function visitor(path: NodePath, logger: Logger, options: AddImportsOptions) { - if (options.adaptor) { - const { name, exports, exportAll } = options.adaptor; - const ignore = - options.ignore?.reduce((obj, key) => { - obj[key] = true; - return obj; - }, {} as Record) ?? {}; + if (options.adaptors) { + const identifiers = findAllDanglingIdentifiers(path.node); - if (name) { - const identifiers = findAllDanglingIdentifiers(path.node); - const usedExports = - exports && exports.length - ? // If we have exports for this adaptor, import any dangling variables from the export list - exports.filter((e) => !ignore[e] && identifiers[e]) - : // If we have no exports for this adaptor, import anything apart from a few choice globals - Object.keys(identifiers).filter( - (i) => !ignore[i] && !globalRE.test(i) - ); - if (usedExports.length) { - // TODO maybe in trace output we can say WHY we're doing these things - addUsedImports(path, usedExports, name); - logger.info(`Added import statement for ${name}`); - if (exportAll) { - addExportAdaptor(path, name); - logger.info(`Added export * statement for ${name}`); + for (const adaptor in options.adaptors) { + const { name, exports, exportAll } = options.adaptors[adaptor]; + const ignore = + options.ignore?.reduce((obj, key) => { + obj[key] = true; + return obj; + }, {} as Record) ?? {}; + + if (name) { + const usedExports = + exports && exports.length + ? // If we have exports for this adaptor, import any dangling variables from the export list + exports.filter((e) => !ignore[e] && identifiers[e]) + : // If we have no exports for this adaptor, import anything apart from a few choice globals + Object.keys(identifiers).filter( + (i) => !ignore[i] && !globalRE.test(i) + ); + if (usedExports.length) { + // TODO maybe in trace output we can say WHY we're doing these things + addUsedImports(path, usedExports, name); + logger.info(`Added import statement for ${name}`); + if (exportAll) { + addExportAdaptor(path, name); + logger.info(`Added export * statement for ${name}`); + } } } } diff --git a/packages/compiler/test/transforms/add-imports.test.ts b/packages/compiler/test/transforms/add-imports.test.ts index 76eccffef..de657ee7d 100644 --- a/packages/compiler/test/transforms/add-imports.test.ts +++ b/packages/compiler/test/transforms/add-imports.test.ts @@ -234,10 +234,12 @@ test('add imports for a test module', async (t) => { const options = { 'add-imports': { - adaptor: { - name: 'test-adaptor', - exports: exports, - }, + adaptors: [ + { + name: 'test-adaptor', + exports: exports, + }, + ], }, }; const transformed = transform(ast, [addImports], options) as n.Program; @@ -260,10 +262,12 @@ test('only add used imports for a test module', async (t) => { const options = { 'add-imports': { - adaptor: { - name: 'test-adaptor', - exports: exports, - }, + adaptors: [ + { + name: 'test-adaptor', + exports: exports, + }, + ], }, }; const transformed = transform(ast, [addImports], options) as n.Program; @@ -285,10 +289,12 @@ test("don't add imports if nothing is used", async (t) => { const options = { 'add-imports': { - adaptor: { - name: 'test-adaptor', - exports: exports, - }, + adaptors: [ + { + name: 'test-adaptor', + exports: exports, + }, + ], }, }; const transformed = transform(ast, [addImports], options) as n.Program; @@ -296,6 +302,47 @@ test("don't add imports if nothing is used", async (t) => { t.assert(transformed.body.length === 0); }); +test('add imports for multiple adaptors', async (t) => { + const ast = b.program([ + b.expressionStatement(b.identifier('x')), + b.expressionStatement(b.identifier('y')), + ]); + + const options = { + 'add-imports': { + adaptors: [ + { + name: 'adaptor-a', + exports: ['x'], + }, + { + name: 'adaptor-b', + exports: ['y'], + }, + ], + }, + }; + const transformed = transform(ast, [addImports], options) as n.Program; + + // Note that the first is y, and the second is x + const [first, second] = transformed.body as [ + n.ImportDeclaration, + n.ImportDeclaration + ]; + t.assert(n.ImportDeclaration.check(first)); + const imports_1 = first.specifiers as n.ImportSpecifier[]; + t.assert(imports_1.length === 1); + t.assert(imports_1.find((i) => i.imported.name === 'y')); + t.is(first.source.value, 'adaptor-b'); + + t.assert(n.ImportDeclaration.check(second)); + const imports_2 = (second as n.ImportDeclaration) + .specifiers as n.ImportSpecifier[]; + t.assert(imports_2.length === 1); + t.assert(imports_2.find((i) => i.imported.name === 'x')); + t.is(second.source.value, 'adaptor-a'); +}); + test("don't import if a variable is declared with the same name", async (t) => { const ast = b.program([ b.variableDeclaration('const', [b.variableDeclarator(b.identifier('x'))]), @@ -307,10 +354,12 @@ test("don't import if a variable is declared with the same name", async (t) => { const options = { 'add-imports': { - adaptor: { - name: 'test-adaptor', - exports: exports, - }, + adaptors: [ + { + name: 'test-adaptor', + exports: exports, + }, + ], }, }; const transformed = transform(ast, [addImports], options) as n.Program; @@ -325,10 +374,12 @@ test('dumbly add imports for an adaptor with empty exports', (t) => { const options = { 'add-imports': { - adaptor: { - name: 'test-adaptor', - exports: [], - }, + adaptors: [ + { + name: 'test-adaptor', + exports: [], + }, + ], }, }; const transformed = transform(ast, [addImports], options) as n.Program; @@ -350,9 +401,11 @@ test('dumbly add imports for an adaptor with unknown exports', (t) => { const options = { 'add-imports': { - adaptor: { - name: 'test-adaptor', - }, + adaptors: [ + { + name: 'test-adaptor', + }, + ], }, }; const transformed = transform(ast, [addImports], options) as n.Program; @@ -405,10 +458,12 @@ test("don't auto add imports for node globals", (t) => { const options = { 'add-imports': { - adaptor: { - name: 'test-adaptor', - exports: [], - }, + adaptors: [ + { + name: 'test-adaptor', + exports: [], + }, + ], }, }; const transformed = transform(ast, [addImports], options) as n.Program; @@ -430,10 +485,12 @@ test("Don't add imports for ignored identifiers", async (t) => { const options = { 'add-imports': { ignore: ['x'], - adaptor: { - name: 'test-adaptor', - exports: [], - }, + adaptors: [ + { + name: 'test-adaptor', + exports: [], + }, + ], }, }; @@ -460,10 +517,12 @@ test("Don't add imports from import specifiers", async (t) => { const options = { 'add-imports': { - adaptor: { - name: 'test-adaptor', - exports: [], - }, + adaptors: [ + { + name: 'test-adaptor', + exports: [], + }, + ], }, }; @@ -480,10 +539,12 @@ test('export everything from an adaptor', (t) => { const options = { 'add-imports': { - adaptor: { - name: 'test-adaptor', - exportAll: true, - }, + adaptors: [ + { + name: 'test-adaptor', + exportAll: true, + }, + ], }, }; const transformed = transform(ast, [addImports], options) as n.Program; From 528e9a0cd2786e80b539ccdc1f983800f38517d2 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Thu, 17 Oct 2024 18:19:57 +0100 Subject: [PATCH 02/52] cli: add support for multiple adaptors on compiler --- .changeset/wild-donuts-check.md | 5 +++ packages/cli/src/compile/compile.ts | 54 ++++++++++++----------- packages/cli/test/compile/compile.test.ts | 10 ++--- 3 files changed, 39 insertions(+), 30 deletions(-) create mode 100644 .changeset/wild-donuts-check.md diff --git a/.changeset/wild-donuts-check.md b/.changeset/wild-donuts-check.md new file mode 100644 index 000000000..741a40180 --- /dev/null +++ b/.changeset/wild-donuts-check.md @@ -0,0 +1,5 @@ +--- +'@openfn/cli': patch +--- + +Support multiple adaptors diff --git a/packages/cli/src/compile/compile.ts b/packages/cli/src/compile/compile.ts index a2f8285f6..50710f25c 100644 --- a/packages/cli/src/compile/compile.ts +++ b/packages/cli/src/compile/compile.ts @@ -115,37 +115,41 @@ export const loadTransformOptions = async ( // If an adaptor is passed in, we need to look up its declared exports // and pass them along to the compiler if (opts.adaptors?.length && opts.ignoreImports != true) { - let exports; - const [pattern] = opts.adaptors; - const [specifier] = pattern.split('='); - - // Preload exports from a path, optionally logging errors in case of a failure - log.debug(`Trying to preload types for ${specifier}`); - const path = await resolveSpecifierPath(pattern, opts.repoDir, log); - if (path) { - try { - exports = await preloadAdaptorExports( - path, - opts.useAdaptorsMonorepo, - log - ); - } catch (e) { - log.error(`Failed to load adaptor typedefs from path ${path}`); - log.error(e); + const adaptorsConfig = []; + for (const adaptorInput of opts.adaptors) { + let exports; + const [specifier] = adaptorInput.split('='); + + // Preload exports from a path, optionally logging errors in case of a failure + log.debug(`Trying to preload types for ${specifier}`); + const path = await resolveSpecifierPath(adaptorInput, opts.repoDir, log); + if (path) { + try { + exports = await preloadAdaptorExports( + path, + opts.useAdaptorsMonorepo, + log + ); + } catch (e) { + log.error(`Failed to load adaptor typedefs from path ${path}`); + log.error(e); + } } - } - if (!exports || exports.length === 0) { - log.debug(`No module exports found for ${pattern}`); - } + if (!exports || exports.length === 0) { + log.debug(`No module exports found for ${adaptorInput}`); + } - options['add-imports'] = { - ignore: opts.ignoreImports as string[], - adaptor: { + adaptorsConfig.push({ name: stripVersionSpecifier(specifier), exports, exportAll: true, - }, + }); + } + + options['add-imports'] = { + ignore: opts.ignoreImports as string[], + adaptors: adaptorsConfig, }; } diff --git a/packages/cli/test/compile/compile.test.ts b/packages/cli/test/compile/compile.test.ts index 46f6520d1..a4b1dcb5d 100644 --- a/packages/cli/test/compile/compile.test.ts +++ b/packages/cli/test/compile/compile.test.ts @@ -20,10 +20,10 @@ const expressionPath = '/job.js'; type TransformOptionsWithImports = { ['add-imports']: { ignore: true | string[]; - adaptor: { + adaptors: Array<{ name: string; exports: string[]; - }; + }>; }; }; @@ -234,7 +234,7 @@ test.serial( t.truthy(result['add-imports']); // Should describe the exports of the times-two module - const { name, exports } = result['add-imports'].adaptor; + const [{ name, exports }] = result['add-imports'].adaptors; t.assert(name === 'times-two'); t.assert(exports.includes('byTwo')); } @@ -257,7 +257,7 @@ test.serial( t.truthy(result['add-imports']); // Should describe the exports of the times-two module - const { name, exports } = result['add-imports'].adaptor; + const [{ name, exports }] = result['add-imports'].adaptors; t.assert(name === 'times-two'); t.assert(exports.includes('byTwo')); } @@ -282,7 +282,7 @@ test.serial( t.truthy(result['add-imports']); // Should describe the exports of the times-two module - const { name, exports } = result['add-imports'].adaptor; + const [{ name, exports }] = result['add-imports'].adaptors; t.assert(name === 'times-two'); t.assert(exports.includes('byTwo')); } From 8673872f97a948dcd9beef6855d116b5075a47e5 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Fri, 18 Oct 2024 09:58:25 +0100 Subject: [PATCH 03/52] compiler: don't extract common exports --- packages/compiler/src/util.ts | 24 +----------------------- 1 file changed, 1 insertion(+), 23 deletions(-) diff --git a/packages/compiler/src/util.ts b/packages/compiler/src/util.ts index 0f70df991..eb5cfc318 100644 --- a/packages/compiler/src/util.ts +++ b/packages/compiler/src/util.ts @@ -24,7 +24,7 @@ export const isRelativeSpecifier = (specifier: string) => // But we may relax this later. export const preloadAdaptorExports = async ( pathToModule: string, - useMonorepo?: boolean, + _useMonorepo?: boolean, log?: Logger ) => { const project = new Project(); @@ -37,28 +37,6 @@ export const preloadAdaptorExports = async ( if (pkg.types) { const functionDefs = {} as Record; - // load common definitions into the project - if (pkg.name !== '@openfn/language-common') { - try { - const common = await findExports( - path.resolve( - pathToModule, - useMonorepo ? '../common' : '../language-common' - ), - 'types/index.d.ts', - project - ); - if (common) { - common.forEach(({ name }) => { - functionDefs[name] = true; - }); - } - } catch (e) { - log?.debug('Failed to load types from language common'); - log?.debug(e); - } - } - const adaptor = await findExports(pathToModule, pkg.types, project); adaptor.forEach(({ name }) => { functionDefs[name] = true; From b6de2c4ffff8d285bd08d425a4e8ee1a127cec4a Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Fri, 18 Oct 2024 11:28:15 +0100 Subject: [PATCH 04/52] compiler: be more specific about how we calculate function exports --- .changeset/spicy-parents-guess.md | 5 ++++ packages/compiler/src/util.ts | 49 +++++++++++++++++++++---------- 2 files changed, 38 insertions(+), 16 deletions(-) create mode 100644 .changeset/spicy-parents-guess.md diff --git a/.changeset/spicy-parents-guess.md b/.changeset/spicy-parents-guess.md new file mode 100644 index 000000000..14d13dbc0 --- /dev/null +++ b/.changeset/spicy-parents-guess.md @@ -0,0 +1,5 @@ +--- +'@openfn/compiler': minor +--- + +Be more accurate in calculating exports from an adaptor" diff --git a/packages/compiler/src/util.ts b/packages/compiler/src/util.ts index eb5cfc318..fa197079e 100644 --- a/packages/compiler/src/util.ts +++ b/packages/compiler/src/util.ts @@ -1,5 +1,5 @@ import { readFileSync } from 'node:fs'; -import { readFile, readdir } from 'node:fs/promises'; +import { readFile } from 'node:fs/promises'; import path from 'node:path'; import { Project, describeDts } from '@openfn/describe-package'; import type { Logger } from '@openfn/logger'; @@ -39,7 +39,9 @@ export const preloadAdaptorExports = async ( const adaptor = await findExports(pathToModule, pkg.types, project); adaptor.forEach(({ name }) => { - functionDefs[name] = true; + if (name !== 'default') { + functionDefs[name] = true; + } }); return Object.keys(functionDefs); @@ -60,20 +62,35 @@ const findExports = async ( types: string, project: Project ) => { - const typesRoot = path.dirname(types); - const files = await readdir(`${moduleRoot}/${typesRoot}`); - const dtsFiles = files.filter((f) => f.endsWith('.d.ts')); - const result = []; - for (const f of dtsFiles) { - const relPath = `${typesRoot}/${f}`; - const contents = await readFile(`${moduleRoot}/${relPath}`, 'utf8'); - project.createFile(contents, relPath); + const results = []; + + const contents = await readFile(`${moduleRoot}/${types}`, 'utf8'); + project.createFile(contents, types); + + results.push( + ...describeDts(project, types, { + includePrivate: true, + }) + ); - result.push( - ...describeDts(project, relPath, { - includePrivate: true, - }) - ); + // Ensure that everything in adaptor.d.ts is exported + // This is kinda cheating but it's quite safe for the time being + const typesRoot = path.dirname(types); + for (const dts of ['adaptor', 'Adaptor']) { + try { + const adaptorPath = `${moduleRoot}/${typesRoot}/${dts}.d.ts`; + const contents = await readFile(adaptorPath, 'utf8'); + project.createFile(contents, adaptorPath); + results.push( + ...describeDts(project, adaptorPath, { + includePrivate: true, + }) + ); + break; + } catch (e) { + // no problem if this throws - likely the file doesn't exist + } } - return result; + + return results; }; From 7245bf72d48ce4fb3620bfebef86756cf159c1fc Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Fri, 18 Oct 2024 16:53:46 +0100 Subject: [PATCH 05/52] remove the adaptors prop --- .changeset/neat-lies-begin.md | 5 ++ .../@openfn/language-postgres_0.0.1/index.js | 3 +- .../language-postgres_0.0.1/types.d.ts | 2 +- packages/engine-multi/src/api/autoinstall.ts | 21 ++++---- packages/engine-multi/src/api/compile.ts | 26 +++++---- packages/engine-multi/src/test/util.ts | 2 +- .../engine-multi/test/api/autoinstall.test.ts | 53 ++++++++++++------- packages/lexicon/core.d.ts | 2 +- 8 files changed, 72 insertions(+), 42 deletions(-) create mode 100644 .changeset/neat-lies-begin.md diff --git a/.changeset/neat-lies-begin.md b/.changeset/neat-lies-begin.md new file mode 100644 index 000000000..2fed34237 --- /dev/null +++ b/.changeset/neat-lies-begin.md @@ -0,0 +1,5 @@ +--- +'@openfn/engine-multi': minor +--- + +Support multiple adaptors in job structures diff --git a/packages/cli/test/__repo__/node_modules/@openfn/language-postgres_0.0.1/index.js b/packages/cli/test/__repo__/node_modules/@openfn/language-postgres_0.0.1/index.js index 8538c25d4..59b37327a 100644 --- a/packages/cli/test/__repo__/node_modules/@openfn/language-postgres_0.0.1/index.js +++ b/packages/cli/test/__repo__/node_modules/@openfn/language-postgres_0.0.1/index.js @@ -1,3 +1,4 @@ export const execute = () => () => 'execute called!'; -export const fn = (f) => (state) => f(state); +// don't use the same functions as common +export const alterState = (f) => (state) => f(state); diff --git a/packages/cli/test/__repo__/node_modules/@openfn/language-postgres_0.0.1/types.d.ts b/packages/cli/test/__repo__/node_modules/@openfn/language-postgres_0.0.1/types.d.ts index b53ff1e00..3a8f37201 100644 --- a/packages/cli/test/__repo__/node_modules/@openfn/language-postgres_0.0.1/types.d.ts +++ b/packages/cli/test/__repo__/node_modules/@openfn/language-postgres_0.0.1/types.d.ts @@ -1,3 +1,3 @@ export declare function execute(f: Array<(state: any) => any>): number; -export declare function fn(f: (state: any) => any): any; +export declare function alterState(f: (state: any) => any): any; diff --git a/packages/engine-multi/src/api/autoinstall.ts b/packages/engine-multi/src/api/autoinstall.ts index 48881e813..8e165d531 100644 --- a/packages/engine-multi/src/api/autoinstall.ts +++ b/packages/engine-multi/src/api/autoinstall.ts @@ -124,7 +124,7 @@ const autoinstall = async (context: ExecutionContext): Promise => { const paths: ModulePaths = {}; const adaptorsToLoad = []; - for (let a of adaptors) { + for (const a of adaptors) { // Ensure that this is not blacklisted if (whitelist && !whitelist.find((r) => r.exec(a))) { // TODO what if it is? For now we'll log and skip it @@ -168,12 +168,13 @@ const autoinstall = async (context: ExecutionContext): Promise => { // Write linker arguments back to the plan for (const step of plan.workflow.steps) { const job = step as unknown as Job; - if (paths[job.adaptor!]) { - const { name } = getNameAndVersion(job.adaptor!); - // @ts-ignore - job.linker = { - [name]: paths[job.adaptor!], - }; + for (const adaptor of job.adaptors ?? []) { + if (paths[adaptor!]) { + const { name } = getNameAndVersion(adaptor!); + job.linker ??= {}; + // @ts-ignore + job.linker[name] = paths[adaptor!]; + } } } @@ -232,8 +233,10 @@ const isInstalled = async ( export const identifyAdaptors = (plan: ExecutionPlan): Set => { const adaptors = new Set(); plan.workflow.steps - .filter((job) => (job as Job).adaptor) - .forEach((job) => adaptors.add((job as Job).adaptor!)); + .filter((job) => (job as Job).adaptors) + .map((job) => (job as Job).adaptors) + .flat() + .forEach((adaptor) => adaptors.add(adaptor as string)); return adaptors; }; diff --git a/packages/engine-multi/src/api/compile.ts b/packages/engine-multi/src/api/compile.ts index c47660adf..151ec103a 100644 --- a/packages/engine-multi/src/api/compile.ts +++ b/packages/engine-multi/src/api/compile.ts @@ -22,7 +22,7 @@ export default async (context: ExecutionContext) => { job.expression as string, logger, repoDir, - job.adaptor // TODO need to expand this. Or do I? + job.adaptors ); } catch (e) { throw new CompileError(e, job.id!); @@ -47,22 +47,30 @@ const compileJob = async ( job: string, logger: Logger, repoDir?: string, - adaptor?: string + adaptors?: string[] ) => { const compilerOptions: Options = { logger, }; - if (adaptor && repoDir) { - // TODO I probably dont want to log this stuff - const pathToAdaptor = await getModulePath(adaptor, repoDir, logger); - const exports = await preloadAdaptorExports(pathToAdaptor!, false, logger); - compilerOptions['add-imports'] = { - adaptor: { + if (adaptors && repoDir) { + const adaptorConfig = []; + for (const adaptor of adaptors) { + // TODO I probably don't want to log this stuff + const pathToAdaptor = await getModulePath(adaptor, repoDir, logger); + const exports = await preloadAdaptorExports( + pathToAdaptor!, + false, + logger + ); + adaptorConfig.push({ name: stripVersionSpecifier(adaptor), exports, exportAll: true, - }, + }); + } + compilerOptions['add-imports'] = { + adaptors: adaptorConfig, }; } return compile(job, compilerOptions); diff --git a/packages/engine-multi/src/test/util.ts b/packages/engine-multi/src/test/util.ts index 494c24e27..93916563a 100644 --- a/packages/engine-multi/src/test/util.ts +++ b/packages/engine-multi/src/test/util.ts @@ -7,7 +7,7 @@ export const createPlan = (job = {}) => steps: [ { id: 'j1', - adaptor: 'common', // not used + adaptors: ['common'], // not used configuration: {}, // not used expression: '(s) => ({ data: { answer: s.data?.input || 42 } })', diff --git a/packages/engine-multi/test/api/autoinstall.test.ts b/packages/engine-multi/test/api/autoinstall.test.ts index ffb288093..1bced2a3a 100644 --- a/packages/engine-multi/test/api/autoinstall.test.ts +++ b/packages/engine-multi/test/api/autoinstall.test.ts @@ -48,7 +48,7 @@ const createContext = ( plan: { workflow: { steps: jobs || [ - { adaptor: '@openfn/language-common@1.0.0', expression: '.' }, + { adaptors: ['@openfn/language-common@1.0.0'], expression: '.' }, ], }, options: {}, @@ -126,15 +126,15 @@ test('identifyAdaptors: pick out adaptors and remove duplicates', (t) => { workflow: { steps: [ { - adaptor: 'common@1.0.0', + adaptors: ['common@1.0.0'], expression: '.', }, { - adaptor: 'common@1.0.0', + adaptors: ['common@1.0.0'], expression: '.', }, { - adaptor: 'common@1.0.1', + adaptors: ['common@1.0.1'], expression: '.', }, ], @@ -150,7 +150,7 @@ test('identifyAdaptors: pick out adaptors and remove duplicates', (t) => { test.serial('autoinstall: handle @latest', async (t) => { const jobs = [ { - adaptor: 'x@latest', + adaptors: ['x@latest'], }, ]; @@ -169,7 +169,7 @@ test.serial('autoinstall: handle @latest', async (t) => { test.serial('autoinstall: handle @next', async (t) => { const jobs = [ { - adaptor: 'x@next', + adaptors: ['x@next'], }, ]; @@ -269,9 +269,15 @@ test.serial('autoinstall: install in sequence', async (t) => { handleIsInstalled: false, } as any; - const c1 = createContext(options, [{ adaptor: '@openfn/language-common@1' }]); - const c2 = createContext(options, [{ adaptor: '@openfn/language-common@2' }]); - const c3 = createContext(options, [{ adaptor: '@openfn/language-common@3' }]); + const c1 = createContext(options, [ + { adaptors: ['@openfn/language-common@1'] }, + ]); + const c2 = createContext(options, [ + { adaptors: ['@openfn/language-common@2'] }, + ]); + const c3 = createContext(options, [ + { adaptors: ['@openfn/language-common@3'] }, + ]); autoinstall(c1); await wait(1); @@ -300,10 +306,10 @@ test('autoinstall: handle two seperate, non-overlapping installs', async (t) => }; const c1 = createContext(options, [ - { adaptor: '@openfn/language-dhis2@1.0.0' }, + { adaptors: ['@openfn/language-dhis2@1.0.0'] }, ]); const c2 = createContext(options, [ - { adaptor: '@openfn/language-http@1.0.0' }, + { adaptors: ['@openfn/language-http@1.0.0'] }, ]); const p1 = await autoinstall(c1); @@ -336,7 +342,7 @@ test.serial( const job = [ { - adaptor: 'lodash@1.0.0', + adaptors: ['lodash@1.0.0'], }, ]; @@ -357,10 +363,10 @@ test.serial( test.serial('autoinstall: return a map to modules', async (t) => { const jobs = [ { - adaptor: '@openfn/language-common@1.0.0', + adaptors: ['@openfn/language-common@1.0.0'], }, { - adaptor: '@openfn/language-http@1.0.0', + adaptors: ['@openfn/language-http@1.0.0'], }, ]; @@ -388,13 +394,16 @@ test.serial('autoinstall: return a map to modules', async (t) => { test.serial('autoinstall: write linker options back to the plan', async (t) => { const jobs = [ { - adaptor: '@openfn/language-common@1.0.0', + adaptors: ['@openfn/language-common@1.0.0'], }, { - adaptor: '@openfn/language-common@2.0.0', + adaptors: [ + '@openfn/language-common@2.0.0', + '@openfn/language-collections@1.0.0', + ], }, { - adaptor: '@openfn/language-http@1.0.0', + adaptors: ['@openfn/language-http@1.0.0'], }, ]; @@ -419,6 +428,10 @@ test.serial('autoinstall: write linker options back to the plan', async (t) => { path: 'tmp/repo/node_modules/@openfn/language-common_2.0.0', version: '2.0.0', }, + '@openfn/language-collections': { + path: 'tmp/repo/node_modules/@openfn/language-collections_1.0.0', + version: '1.0.0', + }, }); t.deepEqual(c.linker, { '@openfn/language-http': { @@ -433,11 +446,11 @@ test.serial('autoinstall: support custom whitelist', async (t) => { const jobs = [ { // will be ignored - adaptor: 'x@1.0.0', + adaptors: ['x@1.0.0'], }, { // will be installed - adaptor: 'y@1.0.0', + adaptors: ['y@1.0.0'], }, ]; @@ -462,7 +475,7 @@ test.serial('autoinstall: emit an event on completion', async (t) => { let event: any; const jobs = [ { - adaptor: '@openfn/language-common@1.0.0', + adaptors: ['@openfn/language-common@1.0.0'], version: '1.0.0', }, ]; diff --git a/packages/lexicon/core.d.ts b/packages/lexicon/core.d.ts index c3c205b11..d87886641 100644 --- a/packages/lexicon/core.d.ts +++ b/packages/lexicon/core.d.ts @@ -27,7 +27,7 @@ export type Workflow = { * This is some openfn expression plus metadata (adaptor, credentials) */ export interface Job extends Step { - adaptor?: string; + adaptors?: string[]; expression: Expression; configuration?: object | string; state?: Omit | string; From ec057fe38d6e1b9f52ed03f1a6c9b8af4d289329 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Fri, 18 Oct 2024 18:46:38 +0100 Subject: [PATCH 06/52] cli: working through adaptor -> adaptors changes Not done yet --- packages/cli/src/compile/compile.ts | 4 +- packages/cli/src/execute/execute.ts | 8 +- packages/cli/src/util/expand-adaptors.ts | 4 +- packages/cli/src/util/load-plan.ts | 38 ++-- .../cli/src/util/map-adaptors-to-monorepo.ts | 4 +- packages/cli/src/util/print-versions.ts | 33 ++-- packages/cli/src/util/validate-plan.ts | 14 +- packages/cli/test/commands.test.ts | 4 +- .../cli/test/execute/parse-adaptors.test.ts | 2 +- .../cli/test/util/expand-adaptors.test.ts | 16 +- packages/cli/test/util/load-plan.test.ts | 61 +++++-- .../util/map-adaptors-to-monorepo.test.ts | 4 +- packages/cli/test/util/validate-plan.test.ts | 166 +++++++++--------- packages/lexicon/core.d.ts | 12 +- 14 files changed, 221 insertions(+), 149 deletions(-) diff --git a/packages/cli/src/compile/compile.ts b/packages/cli/src/compile/compile.ts index 50710f25c..65cf02230 100644 --- a/packages/cli/src/compile/compile.ts +++ b/packages/cli/src/compile/compile.ts @@ -56,10 +56,8 @@ const compileWorkflow = async ( const job = step as Job; const jobOpts = { ...opts, + adaptors: job.adaptors ?? opts.adaptors, }; - if (job.adaptor) { - jobOpts.adaptors = [job.adaptor]; - } if (job.expression) { job.expression = await compileJob( job.expression as string, diff --git a/packages/cli/src/execute/execute.ts b/packages/cli/src/execute/execute.ts index 62286fdb8..f4a21a95b 100644 --- a/packages/cli/src/execute/execute.ts +++ b/packages/cli/src/execute/execute.ts @@ -67,14 +67,12 @@ export function parseAdaptors(plan: ExecutionPlan) { const adaptors: ModuleInfoMap = {}; - // TODO what if there are different versions of the same adaptor? - // This structure can't handle it - we'd need to build it for every job Object.values(plan.workflow.steps).forEach((step) => { const job = step as Job; - if (job.adaptor) { - const { name, ...maybeVersionAndPath } = extractInfo(job.adaptor); + job.adaptors.forEach((adaptor) => { + const { name, ...maybeVersionAndPath } = extractInfo(adaptor); adaptors[name] = maybeVersionAndPath; - } + }); }); return adaptors; diff --git a/packages/cli/src/util/expand-adaptors.ts b/packages/cli/src/util/expand-adaptors.ts index 45b952e9d..9addfd23d 100644 --- a/packages/cli/src/util/expand-adaptors.ts +++ b/packages/cli/src/util/expand-adaptors.ts @@ -26,9 +26,7 @@ export default | ExecutionPlan>( const plan = input as ExecutionPlan; Object.values(plan.workflow.steps).forEach((step) => { const job = step as Job; - if (job.adaptor) { - job.adaptor = expand(job.adaptor); - } + job.adaptors = job.adaptors.map(expand); }); return plan as any; diff --git a/packages/cli/src/util/load-plan.ts b/packages/cli/src/util/load-plan.ts index 4b071d3fb..f46bae301 100644 --- a/packages/cli/src/util/load-plan.ts +++ b/packages/cli/src/util/load-plan.ts @@ -5,7 +5,12 @@ import { isPath } from '@openfn/compiler'; import abort from './abort'; import expandAdaptors from './expand-adaptors'; import mapAdaptorsToMonorepo from './map-adaptors-to-monorepo'; -import type { ExecutionPlan, Job, WorkflowOptions } from '@openfn/lexicon'; +import type { + ExecutionPlan, + Job, + LegacyJob, + WorkflowOptions, +} from '@openfn/lexicon'; import type { Opts } from '../options'; import type { Logger } from './logger'; import type { OldCLIWorkflow } from '../types'; @@ -36,6 +41,7 @@ const loadPlan = async ( const json = await loadJson(jsonPath!, logger); const defaultName = path.parse(jsonPath!).name; + if (json.workflow) { return loadXPlan(json, options, logger, defaultName); } else { @@ -94,15 +100,11 @@ const loadExpression = async ( const expression = await fs.readFile(expressionPath, 'utf8'); const name = path.parse(expressionPath).name; - const step: Job = { expression }; - - // The adaptor should have been expanded nicely already, so we don't need intervene here - if (options.adaptors) { - const [adaptor] = options.adaptors; - if (adaptor) { - step.adaptor = adaptor; - } - } + const step: Job = { + expression, + // The adaptor should have been expanded nicely already, so we don't need intervene here + adaptors: options.adaptors ?? [], + }; const wfOptions: WorkflowOptions = {}; // TODO support state props to remove? @@ -234,6 +236,20 @@ const importExpressions = async ( } }; +// Allow users to specify a single adaptor on a job, +// but convert the internal representation into an array +const ensureAdaptors = (plan: ExecutionPlan) => { + Object.values(plan.workflow.steps).forEach((step) => { + const job = step as LegacyJob; + if (job.adaptor) { + job.adaptors = [job.adaptor]; + delete job.adaptor; + } + // Also, ensure there is an empty adaptors array, which makes everything else easier + job.adaptors ??= []; + }); +}; + const loadXPlan = async ( plan: ExecutionPlan, options: Pick, @@ -247,6 +263,8 @@ const loadXPlan = async ( if (!plan.workflow.name && defaultName) { plan.workflow.name = defaultName; } + ensureAdaptors(plan); + // Note that baseDir should be set up in the default function await importExpressions(plan, options.baseDir!, logger); // expand shorthand adaptors in the workflow jobs diff --git a/packages/cli/src/util/map-adaptors-to-monorepo.ts b/packages/cli/src/util/map-adaptors-to-monorepo.ts index 4e25d9876..d13f5bc33 100644 --- a/packages/cli/src/util/map-adaptors-to-monorepo.ts +++ b/packages/cli/src/util/map-adaptors-to-monorepo.ts @@ -58,9 +58,7 @@ const mapAdaptorsToMonorepo = ( const plan = input as ExecutionPlan; Object.values(plan.workflow.steps).forEach((step) => { const job = step as Job; - if (job.adaptor) { - job.adaptor = updatePath(job.adaptor, monorepoPath, log); - } + job.adaptors = job.adaptors.map((a) => updatePath(a, monorepoPath, log)); }); return plan; diff --git a/packages/cli/src/util/print-versions.ts b/packages/cli/src/util/print-versions.ts index 7532a7ff8..d82075ccb 100644 --- a/packages/cli/src/util/print-versions.ts +++ b/packages/cli/src/util/print-versions.ts @@ -30,14 +30,13 @@ const printVersions = async ( includeComponents = false ) => { const { adaptors, logJson } = options; - let adaptor = ''; - if (adaptors && adaptors.length) { - adaptor = adaptors[0]; - } - let adaptorVersion; - let adaptorName = ''; - if (adaptor) { + let longestAdaptorName = ''; + const adaptorList: Array[] = []; + + adaptors?.forEach((adaptor) => { + let adaptorVersion; + let adaptorName = ''; if (adaptor.match('=')) { const [namePart, pathPart] = adaptor.split('='); adaptorVersion = loadVersionFromPath(pathPart); @@ -50,11 +49,15 @@ const printVersions = async ( adaptorName = name; adaptorVersion = version || 'latest'; } - } + adaptorList.push([adaptorName, adaptorVersion]); + if (adaptorName.length > longestAdaptorName.length) { + longestAdaptorName = adaptorName; + } + }); // Work out the longest label const longest = Math.max( - ...[NODE, CLI, RUNTIME, COMPILER, adaptorName].map((s) => s.length) + ...[NODE, CLI, RUNTIME, COMPILER, longestAdaptorName].map((s) => s.length) ); // Prefix and pad version numbers @@ -83,8 +86,10 @@ const printVersions = async ( output.versions.runtime = runtimeVersion; output.versions.compiler = compilerVersion; } - if (adaptorName) { - output.versions[adaptorName] = adaptorVersion; + if (adaptorList.length) { + for (const [name, version] of adaptorList) { + output.versions[name] = version; + } } } else { output = `Versions: @@ -96,8 +101,10 @@ ${prefix(CLI)}${version}`; ${prefix(COMPILER)}${compilerVersion}`; } - if (adaptorName) { - output += `\n${prefix(adaptorName)}${adaptorVersion}`; + if (adaptorList.length) { + for (const [name, version] of adaptorList) { + output += `\n${prefix(name)}${version}`; + } } } logger.always(output); diff --git a/packages/cli/src/util/validate-plan.ts b/packages/cli/src/util/validate-plan.ts index 87f6d2c49..122ff9286 100644 --- a/packages/cli/src/util/validate-plan.ts +++ b/packages/cli/src/util/validate-plan.ts @@ -1,4 +1,10 @@ -import { ExecutionPlan, Step, WorkflowOptions } from '@openfn/lexicon'; +import { + ExecutionPlan, + Job, + Step, + Trigger, + WorkflowOptions, +} from '@openfn/lexicon'; import { Logger } from '@openfn/logger'; const assertWorkflowStructure = (plan: ExecutionPlan, logger: Logger) => { @@ -23,13 +29,13 @@ const assertWorkflowStructure = (plan: ExecutionPlan, logger: Logger) => { assertOptionsStructure(options, logger); }; -const assertStepStructure = (step: Step, index: number) => { +const assertStepStructure = (step: Job | Trigger, index: number) => { const allowedKeys = [ 'id', 'name', 'next', 'previous', - 'adaptor', + 'adaptors', 'expression', 'state', 'configuration', @@ -42,7 +48,7 @@ const assertStepStructure = (step: Step, index: number) => { } } - if ('adaptor' in step && !('expression' in step)) { + if ((step as Job).adaptors.length && !('expression' in step)) { throw new Error( `Step ${step.id ?? index} with an adaptor must also have an expression` ); diff --git a/packages/cli/test/commands.test.ts b/packages/cli/test/commands.test.ts index cbf69e3e5..d2b1fe863 100644 --- a/packages/cli/test/commands.test.ts +++ b/packages/cli/test/commands.test.ts @@ -169,7 +169,7 @@ test.serial('run test job with default state', async (t) => { t.assert(message === 'Result: 42'); }); -test.serial('run test job with custom state', async (t) => { +test.serial.only('run test job with custom state', async (t) => { const state = JSON.stringify({ data: { answer: 1 } }); await run(`test -S ${state}`, ''); @@ -511,7 +511,7 @@ test.serial( 'use execute from language-postgres: openfn job.js -a @openfn/language-postgres', async (t) => { const job = - 'fn((state) => { /* function isn\t actually called by the mock adaptor */ throw new Error("fake adaptor") });'; + 'alterState((state) => { /* function isn\t actually called by the mock adaptor */ throw new Error("fake adaptor") });'; const result = await run( 'openfn -a @openfn/language-postgres --no-autoinstall', job, diff --git a/packages/cli/test/execute/parse-adaptors.test.ts b/packages/cli/test/execute/parse-adaptors.test.ts index cdbdf6753..6b4a2762e 100644 --- a/packages/cli/test/execute/parse-adaptors.test.ts +++ b/packages/cli/test/execute/parse-adaptors.test.ts @@ -7,7 +7,7 @@ const createPlan = (adaptor: string): ExecutionPlan => ({ workflow: { steps: [ { - adaptor, + adaptors: [adaptor], expression: '.', }, ], diff --git a/packages/cli/test/util/expand-adaptors.test.ts b/packages/cli/test/util/expand-adaptors.test.ts index 23f1a006d..023e70ca6 100644 --- a/packages/cli/test/util/expand-adaptors.test.ts +++ b/packages/cli/test/util/expand-adaptors.test.ts @@ -61,22 +61,22 @@ test('expands adaptors in an execution plan', (t) => { steps: [ { id: 'a', - adaptor: 'common', + adaptors: ['common'], expression: 'fn()', }, { id: 'b', - adaptor: 'http@1.0.0', + adaptors: ['http@1.0.0'], expression: 'fn()', }, { id: 'c', - adaptor: 'salesforce=a/b/c', + adaptors: ['salesforce=a/b/c'], expression: 'fn()', }, { id: 'd', - adaptor: 'a/b/c/my-adaptor.js', + adaptors: ['a/b/c/my-adaptor.js'], expression: 'fn()', }, ], @@ -85,8 +85,8 @@ test('expands adaptors in an execution plan', (t) => { }; expandAdaptors(plan); const [a, b, c, d] = plan.workflow.steps; - t.is(a.adaptor, '@openfn/language-common'); - t.is(b.adaptor, '@openfn/language-http@1.0.0'); - t.is(c.adaptor, '@openfn/language-salesforce=a/b/c'); - t.is(d.adaptor, 'a/b/c/my-adaptor.js'); + t.is(a.adaptors[0], '@openfn/language-common'); + t.is(b.adaptors[0], '@openfn/language-http@1.0.0'); + t.is(c.adaptors[0], '@openfn/language-salesforce=a/b/c'); + t.is(d.adaptors[0], 'a/b/c/my-adaptor.js'); }); diff --git a/packages/cli/test/util/load-plan.test.ts b/packages/cli/test/util/load-plan.test.ts index e2e296e3c..6a44c6a5a 100644 --- a/packages/cli/test/util/load-plan.test.ts +++ b/packages/cli/test/util/load-plan.test.ts @@ -1,7 +1,7 @@ import test from 'ava'; import mock from 'mock-fs'; import { createMockLogger } from '@openfn/logger'; -import type { Job } from '@openfn/lexicon'; +import type { Job, LegacyJob } from '@openfn/lexicon'; import loadPlan from '../../src/util/load-plan'; import { Opts } from '../../src/options'; @@ -12,11 +12,11 @@ const sampleXPlan = { options: { start: 'a' }, workflow: { name: 'wf', - steps: [{ id: 'a', expression: 'x()' }], + steps: [{ id: 'a', expression: 'x()', adaptors: [] }], }, }; -const createPlan = (steps: Job[] = []) => ({ +const createPlan = (steps: Partial[] = []) => ({ workflow: { steps, }, @@ -56,6 +56,7 @@ test.serial('expression: load a plan from an expression.js', async (t) => { t.is(plan.workflow.name, 'job'); t.deepEqual(plan.workflow.steps[0], { expression: 'x', + adaptors: [], }); }); @@ -70,7 +71,7 @@ test.serial('expression: set an adaptor on the plan', async (t) => { const step = plan.workflow.steps[0] as Job; - t.is(step.adaptor, '@openfn/language-common'); + t.is(step.adaptors[0], '@openfn/language-common'); }); test.serial('expression: do not expand adaptors', async (t) => { @@ -85,7 +86,7 @@ test.serial('expression: do not expand adaptors', async (t) => { const step = plan.workflow.steps[0] as Job; - t.is(step.adaptor, 'common'); + t.is(step.adaptors[0], 'common'); }); test.serial('expression: set a timeout on the plan', async (t) => { @@ -147,7 +148,9 @@ test.serial('xplan: expand adaptors', async (t) => { t.truthy(result); const step = result.workflow.steps[0] as Job; - t.is(step.adaptor, '@openfn/language-common@1.0.0'); + t.is(step.adaptors[0], '@openfn/language-common@1.0.0'); + // @ts-ignore + t.is(step.adaptor, undefined); }); test.serial('xplan: do not expand adaptors', async (t) => { @@ -173,7 +176,9 @@ test.serial('xplan: do not expand adaptors', async (t) => { t.truthy(result); const step = result.workflow.steps[0] as Job; - t.is(step.adaptor, 'common@1.0.0'); + t.is(step.adaptors[0], 'common@1.0.0'); + // @ts-ignore + t.is(step.adaptor, undefined); }); test.serial('xplan: set timeout from CLI', async (t) => { @@ -250,7 +255,7 @@ test.serial('xplan: map to monorepo', async (t) => { t.truthy(result); const step = result.workflow.steps[0] as Job; - t.is(step.adaptor, '@openfn/language-common=/repo/packages/common'); + t.is(step.adaptors[0], '@openfn/language-common=/repo/packages/common'); }); test.serial('old-workflow: load a plan from workflow path', async (t) => { @@ -269,6 +274,7 @@ test.serial('old-workflow: load a plan from workflow path', async (t) => { t.deepEqual(plan.workflow.steps[0], { id: 'a', expression: 'x()', + adaptors: [], }); }); @@ -289,8 +295,8 @@ test.serial('step: allow file paths for state', async (t) => { mock({ 'test/state.json': JSON.stringify({ data: { - x: 1 - } + x: 1, + }, }), 'test/wf.json': JSON.stringify(plan), }); @@ -300,7 +306,38 @@ test.serial('step: allow file paths for state', async (t) => { const step = result.workflow.steps[0] as Job; t.deepEqual(step.state, { data: { - x: 1 - } + x: 1, + }, + }); +}); + +test.serial('xplan: support multiple adaptors', async (t) => { + const opts = { + workflowPath: 'test/wf.json', + expandAdaptors: true, + plan: {}, + }; + + const plan = createPlan([ + { + id: 'a', + expression: '.', + adaptors: ['common@1.0.0', '@openfn/language-collections@1.0.0'], + }, + ]); + + mock({ + 'test/wf.json': JSON.stringify(plan), }); + + const result = await loadPlan(opts, logger); + t.truthy(result); + + const step = result.workflow.steps[0] as Job; + t.deepEqual(step.adaptors, [ + '@openfn/language-common@1.0.0', + '@openfn/language-collections@1.0.0', + ]); + // @ts-ignore + t.is(step.adaptor, undefined); }); diff --git a/packages/cli/test/util/map-adaptors-to-monorepo.test.ts b/packages/cli/test/util/map-adaptors-to-monorepo.test.ts index 3c6dd9a7d..97f71e60a 100644 --- a/packages/cli/test/util/map-adaptors-to-monorepo.test.ts +++ b/packages/cli/test/util/map-adaptors-to-monorepo.test.ts @@ -87,7 +87,7 @@ test.serial('mapAdaptorsToMonorepo: map workflow', async (t) => { steps: [ { expression: '.', - adaptor: 'common', + adaptors: ['common'], }, ], }, @@ -99,7 +99,7 @@ test.serial('mapAdaptorsToMonorepo: map workflow', async (t) => { steps: [ { expression: '.', - adaptor: `common=${ABS_REPO_PATH}/packages/common`, + adaptors: [`common=${ABS_REPO_PATH}/packages/common`], }, ], }); diff --git a/packages/cli/test/util/validate-plan.test.ts b/packages/cli/test/util/validate-plan.test.ts index 9d78b2785..81dad64a4 100644 --- a/packages/cli/test/util/validate-plan.test.ts +++ b/packages/cli/test/util/validate-plan.test.ts @@ -6,108 +6,110 @@ import validate from '../../src/util/validate-plan'; const logger = createMockLogger('', { level: 'debug' }); test.afterEach(() => { - logger._reset(); -}) + logger._reset(); +}); test('throws for missing workflow', (t) => { - const plan = { - options: { - start: 'a', - } - } as ExecutionPlan; + const plan = { + options: { + start: 'a', + }, + } as ExecutionPlan; - t.throws(() => validate(plan, logger), { - message: `Missing or invalid "workflow" key in execution plan`, - }); + t.throws(() => validate(plan, logger), { + message: `Missing or invalid "workflow" key in execution plan`, + }); }); test('throws for steps not an array', (t) => { + const plan = { + options: { + start: 'a', + }, + workflow: { + steps: { + id: 'a', + }, + }, + } as unknown as ExecutionPlan; - const plan = { - options: { - start: 'a', - }, - workflow: { - steps: { - id: 'a' - } - }, - } as unknown as ExecutionPlan; - - t.throws(() => validate(plan, logger), { - message: 'The workflow.steps key must be an array', - }); + t.throws(() => validate(plan, logger), { + message: 'The workflow.steps key must be an array', + }); }); test('throws for a step with an adaptor but no expression', (t) => { - const plan = { - options: { - start: 'a', - }, - workflow: { - steps: [ - { - id: 'a', - adaptor: 'z' - } - ], + const plan = { + options: { + start: 'a', + }, + workflow: { + steps: [ + { + id: 'a', + adaptors: ['z'], }, - } as unknown as ExecutionPlan; + ], + }, + } as unknown as ExecutionPlan; - t.throws(() => validate(plan, logger), { - message: 'Step a with an adaptor must also have an expression', - }); + t.throws(() => validate(plan, logger), { + message: 'Step a with an adaptor must also have an expression', + }); }); test('throws for unknown key in a step', (t) => { - const plan = { - options: { - start: 'a', - }, - workflow: { - steps: [ - { - id: 'a', - key: 'z' - } - ], + const plan = { + options: { + start: 'a', + }, + workflow: { + steps: [ + { + id: 'a', + key: 'z', }, - }as unknown as ExecutionPlan; + ], + }, + } as unknown as ExecutionPlan; - t.throws(() => validate(plan, logger), { - message: 'Invalid key "key" in step a', - }); + t.throws(() => validate(plan, logger), { + message: 'Invalid key "key" in step a', + }); }); test.serial('should warn if no steps are defined', (t) => { - const plan: ExecutionPlan = { - options: { - start: 'a', - }, - workflow: { - steps: [], - }, - }; - validate(plan, logger); - const { message, level } = logger._parse(logger._history[0]); - t.is(level, 'warn'); - t.regex(message as string, /The workflow.steps array is empty/); -}) + const plan: ExecutionPlan = { + options: { + start: 'a', + }, + workflow: { + steps: [], + }, + }; + validate(plan, logger); + const { message, level } = logger._parse(logger._history[0]); + t.is(level, 'warn'); + t.regex(message as string, /The workflow.steps array is empty/); +}); test.serial('should warn if unknown key is passed in options', (t) => { - const plan = { - options: { - start: 'a', - key: 'z', + const plan = { + options: { + start: 'a', + key: 'z', + }, + workflow: { + steps: [ + { + id: 'a', + adaptors: [], }, - workflow: { - steps: [{ - id: 'a', - }], - }, - } as unknown as ExecutionPlan; - validate(plan, logger); - const { message, level } = logger._parse(logger._history[0]); - t.is(level, 'warn'); - t.regex(message as string, /Unrecognized option "key" in options object/); -}) + ], + }, + } as unknown as ExecutionPlan; + validate(plan, logger); + const { message, level } = logger._parse(logger._history[0]); + t.is(level, 'warn'); + t.regex(message as string, /Unrecognized option "key" in options object/); +}); diff --git a/packages/lexicon/core.d.ts b/packages/lexicon/core.d.ts index d87886641..a0959bf3d 100644 --- a/packages/lexicon/core.d.ts +++ b/packages/lexicon/core.d.ts @@ -27,7 +27,7 @@ export type Workflow = { * This is some openfn expression plus metadata (adaptor, credentials) */ export interface Job extends Step { - adaptors?: string[]; + adaptors: string[]; expression: Expression; configuration?: object | string; state?: Omit | string; @@ -44,6 +44,16 @@ export interface Job extends Step { >; } +/** + * Defines an older style job from when we only supported a single adaptor + * And mostly we still do, so we want to support the simple adaptor property + * rather than just adaptors + * Internally, this will be mapped + */ +export interface LegacyJob extends Job { + adaptor?: string; +} + /** * A raw openfn-js script to be executed by the runtime * From 81cb45cde830a4e7453df6e645af0881f9c38612 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Mon, 21 Oct 2024 11:35:20 +0100 Subject: [PATCH 07/52] cli: fix remaining tests after refactor --- packages/cli/src/execute/execute.ts | 6 ++++- packages/cli/src/util/validate-plan.ts | 8 +----- packages/cli/test/commands.test.ts | 2 +- .../cli/test/execute/parse-adaptors.test.ts | 27 +++---------------- 4 files changed, 11 insertions(+), 32 deletions(-) diff --git a/packages/cli/src/execute/execute.ts b/packages/cli/src/execute/execute.ts index f4a21a95b..a01cf4d8a 100644 --- a/packages/cli/src/execute/execute.ts +++ b/packages/cli/src/execute/execute.ts @@ -69,7 +69,11 @@ export function parseAdaptors(plan: ExecutionPlan) { Object.values(plan.workflow.steps).forEach((step) => { const job = step as Job; - job.adaptors.forEach((adaptor) => { + // Usually every job should have an adaptors array + // But there are a couple of cases mostly in test, when validation is skipped, + // when the array may not be set + // It's mostly redundant nbut harmless to optionally chain here + job.adaptors?.forEach((adaptor) => { const { name, ...maybeVersionAndPath } = extractInfo(adaptor); adaptors[name] = maybeVersionAndPath; }); diff --git a/packages/cli/src/util/validate-plan.ts b/packages/cli/src/util/validate-plan.ts index 122ff9286..da83a81f1 100644 --- a/packages/cli/src/util/validate-plan.ts +++ b/packages/cli/src/util/validate-plan.ts @@ -1,10 +1,4 @@ -import { - ExecutionPlan, - Job, - Step, - Trigger, - WorkflowOptions, -} from '@openfn/lexicon'; +import { ExecutionPlan, Job, Trigger, WorkflowOptions } from '@openfn/lexicon'; import { Logger } from '@openfn/logger'; const assertWorkflowStructure = (plan: ExecutionPlan, logger: Logger) => { diff --git a/packages/cli/test/commands.test.ts b/packages/cli/test/commands.test.ts index d2b1fe863..0476e76e0 100644 --- a/packages/cli/test/commands.test.ts +++ b/packages/cli/test/commands.test.ts @@ -169,7 +169,7 @@ test.serial('run test job with default state', async (t) => { t.assert(message === 'Result: 42'); }); -test.serial.only('run test job with custom state', async (t) => { +test.serial('run test job with custom state', async (t) => { const state = JSON.stringify({ data: { answer: 1 } }); await run(`test -S ${state}`, ''); diff --git a/packages/cli/test/execute/parse-adaptors.test.ts b/packages/cli/test/execute/parse-adaptors.test.ts index 6b4a2762e..8bbb0efdd 100644 --- a/packages/cli/test/execute/parse-adaptors.test.ts +++ b/packages/cli/test/execute/parse-adaptors.test.ts @@ -68,22 +68,22 @@ test('parse plan with several steps', (t) => { workflow: { steps: [ { - adaptor: '@openfn/language-common', + adaptors: ['@openfn/language-common'], expression: 'fn()', }, { - adaptor: '@openfn/language-http@1.0.0', + adaptors: ['@openfn/language-http@1.0.0'], expression: 'fn()', }, { - adaptor: '@openfn/language-salesforce=a/b/c', + adaptors: ['@openfn/language-salesforce=a/b/c'], expression: 'fn()', }, ], }, }; const result = parseAdaptors(plan); - t.assert(Object.keys(result).length === 3); + t.is(Object.keys(result).length, 3); t.deepEqual(result, { '@openfn/language-common': {}, '@openfn/language-http': { @@ -94,22 +94,3 @@ test('parse plan with several steps', (t) => { }, }); }); - -// TODO we can't do this right now -// We'd have to send different maps to different jobs -// Which we can support but maybe I'm gonna push that out of scope -test.skip('parse workflow with multiple versions of the same adaptor', (t) => { - const workflow = { - start: 'a', - jobs: { - a: { - adaptor: '@openfn/language-common@1.0.0', - expression: 'fn()', - }, - b: { - adaptor: '@openfn/language-common@2.0.0', - expression: 'fn()', - }, - }, - }; -}); From b15f151350ff8fdfe958e5660f2bb821177fa381 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Mon, 21 Oct 2024 14:09:51 +0100 Subject: [PATCH 08/52] worker: refactor to support adaptors array --- .changeset/sharp-needles-peel.md | 5 + packages/lexicon/lightning.d.ts | 23 ++- .../src/util/convert-lightning-plan.ts | 10 +- packages/ws-worker/test/api/execute.test.ts | 2 +- packages/ws-worker/test/channels/run.test.ts | 2 +- packages/ws-worker/test/lightning.test.ts | 161 ++++++++---------- .../test/mock/runtime-engine.test.ts | 8 +- packages/ws-worker/test/reasons.test.ts | 2 +- packages/ws-worker/test/util.ts | 8 +- .../test/util/convert-lightning-plan.test.ts | 12 +- packages/ws-worker/test/worker.test.ts | 15 +- 11 files changed, 126 insertions(+), 122 deletions(-) create mode 100644 .changeset/sharp-needles-peel.md diff --git a/.changeset/sharp-needles-peel.md b/.changeset/sharp-needles-peel.md new file mode 100644 index 000000000..2a010c43f --- /dev/null +++ b/.changeset/sharp-needles-peel.md @@ -0,0 +1,5 @@ +--- +'@openfn/ws-worker': patch +--- + +Update worker to use adaptors as an array on xplans. Internal only change. diff --git a/packages/lexicon/lightning.d.ts b/packages/lexicon/lightning.d.ts index a30ba111c..857eacf13 100644 --- a/packages/lexicon/lightning.d.ts +++ b/packages/lexicon/lightning.d.ts @@ -1,5 +1,5 @@ import type { SanitizePolicies } from '@openfn/logger'; -import { State } from './core'; +import { LegacyJob, State } from './core'; export const API_VERSION: number; @@ -29,9 +29,9 @@ export type LightningPlan = { dataclip_id: string; starting_node_id: string; - triggers: Node[]; - jobs: Node[]; - edges: Edge[]; + triggers: LightningTrigger[]; + jobs: LightningJob[]; + edges: LightningEdge[]; options?: LightningPlanOptions; }; @@ -59,16 +59,23 @@ export type LightningPlanOptions = { * Sticking with the Node/Edge semantics to help distinguish the * Lightning and runtime typings */ -export type Node = { +export interface LightningNode { id: string; name?: string; body?: string; adaptor?: string; credential?: any; credential_id?: string; - type?: 'webhook' | 'cron'; // trigger only state?: State; -}; +} + +export interface LightningTrigger extends LightningNode { + type: 'webhook' | 'cron'; +} + +export interface LightningJob extends LightningNode { + adaptor: string; +} /** * This is a Path (or link) between two Jobs in a Plan. @@ -76,7 +83,7 @@ export type Node = { * Sticking with the Node/Edge semantics to help distinguish the * Lightning and runtime typings */ -export interface Edge { +export interface LightningEdge { id: string; source_job_id?: string; source_trigger_id?: string; diff --git a/packages/ws-worker/src/util/convert-lightning-plan.ts b/packages/ws-worker/src/util/convert-lightning-plan.ts index df8d4b96c..eff47d12a 100644 --- a/packages/ws-worker/src/util/convert-lightning-plan.ts +++ b/packages/ws-worker/src/util/convert-lightning-plan.ts @@ -10,7 +10,7 @@ import type { WorkflowOptions, Lazy, } from '@openfn/lexicon'; -import { LightningPlan, Edge } from '@openfn/lexicon/lightning'; +import { LightningPlan, LightningEdge } from '@openfn/lexicon/lightning'; import { ExecuteOptions } from '@openfn/engine-multi'; export const conditions: Record string | null> = @@ -22,7 +22,7 @@ export const conditions: Record string | null> = always: (_upstreamId: string) => null, }; -const mapEdgeCondition = (edge: Edge) => { +const mapEdgeCondition = (edge: LightningEdge) => { const { condition } = edge; if (condition && condition in conditions) { const upstream = (edge.source_job_id || edge.source_trigger_id) as string; @@ -31,7 +31,7 @@ const mapEdgeCondition = (edge: Edge) => { return condition; }; -const mapTriggerEdgeCondition = (edge: Edge) => { +const mapTriggerEdgeCondition = (edge: LightningEdge) => { const { condition } = edge; // This handles cron triggers with undefined conditions and the 'always' string. if (condition === undefined || condition === 'always') return true; @@ -88,7 +88,7 @@ export default ( const nodes: Record = {}; - const edges: Edge[] = run.edges ?? []; + const edges: LightningEdge[] = run.edges ?? []; // We don't really care about triggers, it's mostly just a empty node if (run.triggers?.length) { @@ -125,7 +125,7 @@ export default ( id, configuration: step.credential || step.credential_id, expression: step.body!, - adaptor: step.adaptor, + adaptors: step.adaptor ? [step.adaptor] : [], }; if (step.name) { diff --git a/packages/ws-worker/test/api/execute.test.ts b/packages/ws-worker/test/api/execute.test.ts index 16ccdf089..926804630 100644 --- a/packages/ws-worker/test/api/execute.test.ts +++ b/packages/ws-worker/test/api/execute.test.ts @@ -459,7 +459,7 @@ test('execute should call all events on the socket', async (t) => { { id: 'trigger', configuration: 'a', - adaptor: '@openfn/language-common@1.0.0', + adaptors: ['@openfn/language-common@1.0.0'], expression: 'fn(() => console.log("x"))', }, ], diff --git a/packages/ws-worker/test/channels/run.test.ts b/packages/ws-worker/test/channels/run.test.ts index 37dfa4d72..4bbe192b6 100644 --- a/packages/ws-worker/test/channels/run.test.ts +++ b/packages/ws-worker/test/channels/run.test.ts @@ -42,7 +42,7 @@ test('loadRun should return an execution plan and options', async (t) => { id: 'job-1', configuration: 'a', expression: 'fn(a => a)', - adaptor: '@openfn/language-common@1.0.0', + adaptors: ['@openfn/language-common@1.0.0'], }, ], }, diff --git a/packages/ws-worker/test/lightning.test.ts b/packages/ws-worker/test/lightning.test.ts index 42ed02230..4d9b08924 100644 --- a/packages/ws-worker/test/lightning.test.ts +++ b/packages/ws-worker/test/lightning.test.ts @@ -49,7 +49,7 @@ test.before(async () => { backoff: { min: 1, max: 1000, - } + }, }); }); @@ -110,109 +110,98 @@ test.serial( } ); -test.serial( - `should not claim while at capacity, then resume`, - (t) => { - return new Promise((done) => { - - let runIsActive = false; - let runComplete = false; - let didClaimAfterComplete = false; +test.serial(`should not claim while at capacity, then resume`, (t) => { + return new Promise((done) => { + let runIsActive = false; + let runComplete = false; + let didClaimAfterComplete = false; - const run = { - id: `a${++rollingRunId}`, - jobs: [ - { - id: 'j', - adaptor: '@openfn/language-common@1.0.0', - body: `fn(() => new Promise((resolve) => { + const run = { + id: `a${++rollingRunId}`, + jobs: [ + { + id: 'j', + adaptor: '@openfn/language-common@1.0.0', + body: `fn(() => new Promise((resolve) => { setTimeout(resolve, 500) }))`, - }, - ], - }; - - - lng.on(e.CLAIM, () => { - if (runIsActive) { - t.fail('Claimed while run is active') - } - if (runComplete) { - didClaimAfterComplete = true; - } - }); + }, + ], + }; - lng.onSocketEvent(e.RUN_START, run.id, () => { - runIsActive = true; - }) + lng.on(e.CLAIM, () => { + if (runIsActive) { + t.fail('Claimed while run is active'); + } + if (runComplete) { + didClaimAfterComplete = true; + } + }); - lng.onSocketEvent(e.RUN_COMPLETE, run.id, () => { - runIsActive = false; - runComplete = true; + lng.onSocketEvent(e.RUN_START, run.id, () => { + runIsActive = true; + }); - setTimeout(() => { - t.true(didClaimAfterComplete); - done() - }, 10) - }); + lng.onSocketEvent(e.RUN_COMPLETE, run.id, () => { + runIsActive = false; + runComplete = true; - lng.enqueueRun(run); + setTimeout(() => { + t.true(didClaimAfterComplete); + done(); + }, 10); }); - } -); -test.serial( - `should reset backoff after claim`, - (t) => { - return new Promise((done) => { + lng.enqueueRun(run); + }); +}); - let lastClaim = Date.now() - let lastClaimDiff = 0; +test.serial(`should reset backoff after claim`, (t) => { + return new Promise((done) => { + let lastClaim = Date.now(); + let lastClaimDiff = 0; - const run = { - id: `a${++rollingRunId}`, - jobs: [ - { - id: 'j', - adaptor: '@openfn/language-common@1.0.0', - body: `fn(() => new Promise((resolve) => { + const run = { + id: `a${++rollingRunId}`, + jobs: [ + { + id: 'j', + adaptor: '@openfn/language-common@1.0.0', + body: `fn(() => new Promise((resolve) => { setTimeout(resolve, 500) }))`, - }, - ], - }; - + }, + ], + }; - lng.on(e.CLAIM, () => { - lastClaimDiff = Date.now() - lastClaim; - lastClaim = Date.now() - }); + lng.on(e.CLAIM, () => { + lastClaimDiff = Date.now() - lastClaim; + lastClaim = Date.now(); + }); - lng.onSocketEvent(e.RUN_COMPLETE, run.id, () => { - // set this articially high - if there are no more claims, the test will fail - lastClaimDiff = 10000; - - // When the run is finished, the claims should resume - // but with a smaller backoff - setTimeout(() => { - t.log('Backoff after run:', lastClaimDiff) - t.true(lastClaimDiff < 5) - done() - }, 10) - }); + lng.onSocketEvent(e.RUN_COMPLETE, run.id, () => { + // set this articially high - if there are no more claims, the test will fail + lastClaimDiff = 10000; - + // When the run is finished, the claims should resume + // but with a smaller backoff setTimeout(() => { - t.log('Backoff before run:', lastClaimDiff) - // let the backoff increase a bit - // the last claim diff should be at least 30ms - t.true(lastClaimDiff > 30) - - lng.enqueueRun(run); - }, 600) + t.log('Backoff after run:', lastClaimDiff); + t.true(lastClaimDiff < 5); + done(); + }, 10); }); - } -); + + setTimeout(() => { + t.log('Backoff before run:', lastClaimDiff); + // let the backoff increase a bit + // the last claim diff should be at least 20ms + t.true(lastClaimDiff > 20); + + lng.enqueueRun(run); + }, 600); + }); +}); test.todo('worker should log when a run token is verified'); diff --git a/packages/ws-worker/test/mock/runtime-engine.test.ts b/packages/ws-worker/test/mock/runtime-engine.test.ts index aa66c2aa6..28842769e 100644 --- a/packages/ws-worker/test/mock/runtime-engine.test.ts +++ b/packages/ws-worker/test/mock/runtime-engine.test.ts @@ -17,7 +17,7 @@ const sampleWorkflow = { steps: [ { id: 'j1', - adaptor: 'common@1.0.0', + adaptors: ['common@1.0.0'], expression: 'fn(() => ({ data: { x: 10 } }))', }, ], @@ -85,7 +85,7 @@ test.serial('Dispatch complete events for a job', async (t) => { test.serial('Dispatch error event for a crash', async (t) => { const wf = createPlan({ id: 'j1', - adaptor: 'common@1.0.0', + adaptors: ['common@1.0.0'], expression: 'fn(() => ( @~!"@£!4 )', }); @@ -146,7 +146,7 @@ test.serial('listen to events', async (t) => { const wf = createPlan({ id: 'j1', - adaptor: 'common@1.0.0', + adaptors: ['common@1.0.0'], expression: 'export default [() => { console.log("x"); }]', }); @@ -236,7 +236,7 @@ test.serial( // @ts-ignore const workflow = createPlan({ id: 'j1', - adaptor: '@openfn/language-common@1.0.0', + adaptors: ['@openfn/language-common@1.0.0'], }); let didCallEvent = false; diff --git a/packages/ws-worker/test/reasons.test.ts b/packages/ws-worker/test/reasons.test.ts index c66c2ef40..018776aea 100644 --- a/packages/ws-worker/test/reasons.test.ts +++ b/packages/ws-worker/test/reasons.test.ts @@ -209,7 +209,7 @@ test('exception: autoinstall error', async (t) => { const plan = createPlan({ id: 'a', expression: '.', - adaptor: '@openfn/language-common@1.0.0', + adaptors: ['@openfn/language-common@1.0.0'], }); // TODO I also need to ensure that this calls run:complete diff --git a/packages/ws-worker/test/util.ts b/packages/ws-worker/test/util.ts index df70a3c99..d29aa3e5b 100644 --- a/packages/ws-worker/test/util.ts +++ b/packages/ws-worker/test/util.ts @@ -1,5 +1,5 @@ import { ExecutionPlan, Job } from '@openfn/lexicon'; -import { Edge, Node } from '@openfn/lexicon/lightning'; +import { LightningEdge, LightningNode } from '@openfn/lexicon/lightning'; import crypto from 'node:crypto'; export const wait = (fn: () => any, maxRuns = 100) => @@ -34,7 +34,7 @@ export const sleep = (delay = 100) => setTimeout(resolve, delay); }); -export const createPlan = (...steps: Job[]) => +export const createPlan = (...steps: Partial[]) => ({ id: crypto.randomUUID(), workflow: { @@ -48,13 +48,13 @@ export const createEdge = (from: string, to: string) => id: `${from}-${to}`, source_job_id: from, target_job_id: to, - } as Edge); + } as LightningEdge); export const createJob = (body?: string, id?: string) => ({ id: id || crypto.randomUUID(), body: body || `fn((s) => s)`, - } as Node); + } as LightningNode); export const createRun = (jobs = [], edges = [], triggers = []) => ({ id: crypto.randomUUID(), diff --git a/packages/ws-worker/test/util/convert-lightning-plan.test.ts b/packages/ws-worker/test/util/convert-lightning-plan.test.ts index 0d3ef653a..79148ea2d 100644 --- a/packages/ws-worker/test/util/convert-lightning-plan.test.ts +++ b/packages/ws-worker/test/util/convert-lightning-plan.test.ts @@ -1,5 +1,9 @@ import test from 'ava'; -import type { LightningPlan, Node } from '@openfn/lexicon/lightning'; +import type { + LightningPlan, + LightningJob, + LightningTrigger, +} from '@openfn/lexicon/lightning'; import convertPlan, { conditions } from '../../src/util/convert-lightning-plan'; import { ConditionalStepEdge, Job } from '@openfn/lexicon'; @@ -11,7 +15,7 @@ const createNode = (props = {}) => adaptor: 'common', credential_id: 'y', ...props, - } as Node); + } as LightningJob); const createEdge = (from: string, to: string, props = {}) => ({ id: `${from}-${to}`, @@ -26,13 +30,13 @@ const createTrigger = (props = {}) => id: 't', type: 'cron', ...props, - } as Node); + } as LightningTrigger); // Creates a runtime job node const createJob = (props = {}) => ({ id: 'a', expression: 'x', - adaptor: 'common', + adaptors: ['common'], configuration: 'y', ...props, }); diff --git a/packages/ws-worker/test/worker.test.ts b/packages/ws-worker/test/worker.test.ts index 19f4fc5c6..5a18f79d7 100644 --- a/packages/ws-worker/test/worker.test.ts +++ b/packages/ws-worker/test/worker.test.ts @@ -44,7 +44,7 @@ const execute = async (plan: ExecutionPlan, input = {}, options = {}) => [STEP_START]: async () => true, [RUN_LOG]: async (_evt) => { //console.log(evt.source, evt.message) - return true + return true; }, [STEP_COMPLETE]: async () => true, [RUN_COMPLETE]: async () => true, @@ -60,7 +60,6 @@ const execute = async (plan: ExecutionPlan, input = {}, options = {}) => doExecute(channel, engine, logger, plan, input, options, onFinish); }); - // Repro for https://github.com/OpenFn/kit/issues/616 // This will not run in CI unless the env is set if (process.env.OPENFN_TEST_SF_TOKEN && process.env.OPENFN_TEST_SF_PASSWORD) { @@ -80,21 +79,21 @@ if (process.env.OPENFN_TEST_SF_TOKEN && process.env.OPENFN_TEST_SF_PASSWORD) { "email": "test@test.com" }]) )`, - adaptor: '@openfn/language-salesforce@4.5.0', + adaptors: ['@openfn/language-salesforce@4.5.0'], configuration: { username: 'demo@openfn.org', securityToken: process.env.OPENFN_TEST_SF_TOKEN, password: process.env.OPENFN_TEST_SF_PASSWORD, loginUrl: 'https://login.salesforce.com', - } + }, }); const input = { data: { result: 42 } }; - const result= await execute(plan, input); - t.log(result) + const result = await execute(plan, input); + t.log(result); // Actually this fails right but it's a permissions thing on the sandbox t.is(result.reason.reason, 'success'); - }) -} \ No newline at end of file + }); +} From a6c3149585c36c13ba949751da1fc111aaf8de06 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Mon, 21 Oct 2024 15:02:06 +0100 Subject: [PATCH 09/52] various: yet another type restructure to handle adaptors safely --- .../src/execute/get-autoinstall-targets.ts | 8 +++-- packages/cli/src/types.ts | 20 ++++++++--- packages/cli/src/util/expand-adaptors.ts | 6 ++-- packages/cli/src/util/load-plan.ts | 23 +++++-------- .../cli/src/util/map-adaptors-to-monorepo.ts | 6 +++- packages/cli/src/util/validate-plan.ts | 2 +- .../execute/get-autoinstall-targets.test.ts | 34 ++++++++++++++----- packages/engine-multi/test/engine.test.ts | 1 + packages/lexicon/core.d.ts | 21 ++++-------- packages/runtime/src/execute/compile-plan.ts | 9 +++-- packages/runtime/src/util/validate-plan.ts | 10 +++--- .../runtime/test/execute/compile-plan.test.ts | 4 +-- packages/runtime/test/runtime.test.ts | 6 ++-- 13 files changed, 88 insertions(+), 62 deletions(-) diff --git a/packages/cli/src/execute/get-autoinstall-targets.ts b/packages/cli/src/execute/get-autoinstall-targets.ts index 677f41f50..aea294366 100644 --- a/packages/cli/src/execute/get-autoinstall-targets.ts +++ b/packages/cli/src/execute/get-autoinstall-targets.ts @@ -5,9 +5,11 @@ const getAutoinstallTargets = (plan: ExecutionPlan) => { Object.values(plan.workflow.steps).forEach((step) => { const job = step as Job; // Do not autoinstall adaptors with a path - if (job.adaptor && !/=/.test(job.adaptor)) { - adaptors[job.adaptor] = true; - } + job.adaptors + ?.filter((adaptor) => !/=/.test(adaptor)) + .forEach((adaptor) => { + adaptors[adaptor] = true; + }); }); return Object.keys(adaptors); }; diff --git a/packages/cli/src/types.ts b/packages/cli/src/types.ts index ed27ef8bc..1ca6ad63c 100644 --- a/packages/cli/src/types.ts +++ b/packages/cli/src/types.ts @@ -1,4 +1,7 @@ // the executionPLan for the CLI is a bit differnt to the runtime's perspective + +import { Trigger, UUID, WorkflowOptions } from '@openfn/lexicon'; + // Ie config can be a string export type JobNodeID = string; @@ -9,19 +12,26 @@ export type OldCLIWorkflow = { }; export type CLIExecutionPlan = { - id?: string; // UUID for this plan - start?: JobNodeID; - jobs: CLIJobNode[]; + id?: string; + options: WorkflowOptions; + workflow: { + id?: UUID; + name?: string; + steps: Array; + }; }; export type CLIJobNode = { id?: string; - adaptor?: string; expression?: string; // path or expression configuration?: string | object; // path or credential object data?: any; - next?: string | Record; + + // We can accept a single adaptor or multiple + // The CLI will convert it to adaptors as an array + adaptor?: string; + adaptors?: string[]; }; export type CLIJobEdge = { diff --git a/packages/cli/src/util/expand-adaptors.ts b/packages/cli/src/util/expand-adaptors.ts index 9addfd23d..3ccd72fd1 100644 --- a/packages/cli/src/util/expand-adaptors.ts +++ b/packages/cli/src/util/expand-adaptors.ts @@ -14,8 +14,6 @@ const expand = (name: string) => { type ArrayOrPlan = T extends string[] ? string[] : ExecutionPlan; -// TODO typings here aren't good,I can't get this to work! -// At least this looks nice externally export default | ExecutionPlan>( input: T ): ArrayOrPlan => { @@ -26,7 +24,9 @@ export default | ExecutionPlan>( const plan = input as ExecutionPlan; Object.values(plan.workflow.steps).forEach((step) => { const job = step as Job; - job.adaptors = job.adaptors.map(expand); + if (job.adaptors) { + job.adaptors = job.adaptors.map(expand); + } }); return plan as any; diff --git a/packages/cli/src/util/load-plan.ts b/packages/cli/src/util/load-plan.ts index f46bae301..ed12eb8f4 100644 --- a/packages/cli/src/util/load-plan.ts +++ b/packages/cli/src/util/load-plan.ts @@ -5,15 +5,10 @@ import { isPath } from '@openfn/compiler'; import abort from './abort'; import expandAdaptors from './expand-adaptors'; import mapAdaptorsToMonorepo from './map-adaptors-to-monorepo'; -import type { - ExecutionPlan, - Job, - LegacyJob, - WorkflowOptions, -} from '@openfn/lexicon'; +import type { ExecutionPlan, Job, WorkflowOptions } from '@openfn/lexicon'; import type { Opts } from '../options'; import type { Logger } from './logger'; -import type { OldCLIWorkflow } from '../types'; +import type { CLIExecutionPlan, CLIJobNode, OldCLIWorkflow } from '../types'; const loadPlan = async ( options: Pick< @@ -110,7 +105,7 @@ const loadExpression = async ( // TODO support state props to remove? maybeAssign(options, wfOptions, ['timeout']); - const plan: ExecutionPlan = { + const plan: CLIExecutionPlan = { workflow: { name, steps: [step], @@ -128,7 +123,7 @@ const loadExpression = async ( ); // This will never execute - return {} as ExecutionPlan; + return {} as CLIExecutionPlan; } }; @@ -190,7 +185,7 @@ const fetchFile = async ( // TODO this is currently untested in load-plan // (but covered a bit in execute tests) const importExpressions = async ( - plan: ExecutionPlan, + plan: CLIExecutionPlan, rootDir: string, log: Logger ) => { @@ -238,9 +233,9 @@ const importExpressions = async ( // Allow users to specify a single adaptor on a job, // but convert the internal representation into an array -const ensureAdaptors = (plan: ExecutionPlan) => { +const ensureAdaptors = (plan: CLIExecutionPlan) => { Object.values(plan.workflow.steps).forEach((step) => { - const job = step as LegacyJob; + const job = step as CLIJobNode; if (job.adaptor) { job.adaptors = [job.adaptor]; delete job.adaptor; @@ -251,7 +246,7 @@ const ensureAdaptors = (plan: ExecutionPlan) => { }; const loadXPlan = async ( - plan: ExecutionPlan, + plan: CLIExecutionPlan, options: Pick, logger: Logger, defaultName: string = '' @@ -279,5 +274,5 @@ const loadXPlan = async ( logger.info(`Loaded workflow ${plan.workflow.name ?? ''}`); - return plan; + return plan as ExecutionPlan; }; diff --git a/packages/cli/src/util/map-adaptors-to-monorepo.ts b/packages/cli/src/util/map-adaptors-to-monorepo.ts index d13f5bc33..8154a124f 100644 --- a/packages/cli/src/util/map-adaptors-to-monorepo.ts +++ b/packages/cli/src/util/map-adaptors-to-monorepo.ts @@ -58,7 +58,11 @@ const mapAdaptorsToMonorepo = ( const plan = input as ExecutionPlan; Object.values(plan.workflow.steps).forEach((step) => { const job = step as Job; - job.adaptors = job.adaptors.map((a) => updatePath(a, monorepoPath, log)); + if (job.adaptors) { + job.adaptors = job.adaptors.map((a) => + updatePath(a, monorepoPath, log) + ); + } }); return plan; diff --git a/packages/cli/src/util/validate-plan.ts b/packages/cli/src/util/validate-plan.ts index da83a81f1..b966e101a 100644 --- a/packages/cli/src/util/validate-plan.ts +++ b/packages/cli/src/util/validate-plan.ts @@ -42,7 +42,7 @@ const assertStepStructure = (step: Job | Trigger, index: number) => { } } - if ((step as Job).adaptors.length && !('expression' in step)) { + if ((step as Job).adaptors?.length && !('expression' in step)) { throw new Error( `Step ${step.id ?? index} with an adaptor must also have an expression` ); diff --git a/packages/cli/test/execute/get-autoinstall-targets.test.ts b/packages/cli/test/execute/get-autoinstall-targets.test.ts index 33a29786b..38e525346 100644 --- a/packages/cli/test/execute/get-autoinstall-targets.test.ts +++ b/packages/cli/test/execute/get-autoinstall-targets.test.ts @@ -29,14 +29,14 @@ test('plan with zero adaptors', (t) => { t.is(result.length, 0); }); -test('plan with multiple adaptors', (t) => { +test('plan with multiple adaptors in multiple steps', (t) => { const plan = getPlan([ { - adaptor: '@openfn/language-common', + adaptors: ['@openfn/language-common'], expression: 'fn()', }, { - adaptor: '@openfn/language-http', + adaptors: ['@openfn/language-http'], expression: 'fn()', }, ]); @@ -48,11 +48,11 @@ test('plan with multiple adaptors', (t) => { test('plan with duplicate adaptors', (t) => { const plan = getPlan([ { - adaptor: '@openfn/language-common', + adaptors: ['@openfn/language-common'], expression: 'fn()', }, { - adaptor: '@openfn/language-common', + adaptors: ['@openfn/language-common'], expression: 'fn()', }, ]); @@ -61,18 +61,34 @@ test('plan with duplicate adaptors', (t) => { t.deepEqual(result, ['@openfn/language-common']); }); +test('plan with multiple adaptors in one step with duplicates', (t) => { + const plan = getPlan([ + { + adaptors: [ + '@openfn/language-common', + '@openfn/language-http', + '@openfn/language-http', + ], + expression: 'fn()', + }, + ]); + const result = getAutoinstallTargets(plan); + t.is(result.length, 2); + t.deepEqual(result, ['@openfn/language-common', '@openfn/language-http']); +}); + test('plan with one adaptor but different versions', (t) => { const plan = getPlan([ { - adaptor: '@openfn/language-common@1.0.0', + adaptors: ['@openfn/language-common@1.0.0'], expression: 'fn()', }, { - adaptor: '@openfn/language-common@2.0.0', + adaptors: ['@openfn/language-common@2.0.0'], expression: 'fn()', }, { - adaptor: '@openfn/language-common@3.0.0', + adaptors: ['@openfn/language-common@3.0.0'], expression: 'fn()', }, ]); @@ -89,7 +105,7 @@ test('do not return adaptors with a path', (t) => { const plan = getPlan([ { expression: 'fn()', - adaptor: 'common=a/b/c', + adaptors: ['common=a/b/c'], }, ]); const result = getAutoinstallTargets(plan); diff --git a/packages/engine-multi/test/engine.test.ts b/packages/engine-multi/test/engine.test.ts index d93c85f62..e1dbec616 100644 --- a/packages/engine-multi/test/engine.test.ts +++ b/packages/engine-multi/test/engine.test.ts @@ -28,6 +28,7 @@ const createPlan = (expression: string = '.', id = 'a') => ({ steps: [ { expression, + adaptors: [], }, ], }, diff --git a/packages/lexicon/core.d.ts b/packages/lexicon/core.d.ts index a0959bf3d..f5626a546 100644 --- a/packages/lexicon/core.d.ts +++ b/packages/lexicon/core.d.ts @@ -3,11 +3,14 @@ import { SanitizePolicies } from '@openfn/logger'; /** * An execution plan is a portable definition of a Work Order, * or, a unit of work to execute + * This definition represents the external format - the shape of + * the plan pre-compilation before it's passed into the runtime manager + * (ie, the CLI or Worker) */ export type ExecutionPlan = { id?: UUID; // this would be the run (nee attempt) id workflow: Workflow; - options: WorkflowOptions; + options?: WorkflowOptions; }; /** @@ -27,14 +30,14 @@ export type Workflow = { * This is some openfn expression plus metadata (adaptor, credentials) */ export interface Job extends Step { - adaptors: string[]; + adaptors?: string[]; expression: Expression; configuration?: object | string; state?: Omit | string; // Internal use only - // Allow module paths and versions to be overriden in the linker - // Maps to runtime.ModuleInfoMapo + // Allow module paths and versions to be overridden in the linker + // Maps to runtime.ModuleInfoMap linker?: Record< string, { @@ -44,16 +47,6 @@ export interface Job extends Step { >; } -/** - * Defines an older style job from when we only supported a single adaptor - * And mostly we still do, so we want to support the simple adaptor property - * rather than just adaptors - * Internally, this will be mapped - */ -export interface LegacyJob extends Job { - adaptor?: string; -} - /** * A raw openfn-js script to be executed by the runtime * diff --git a/packages/runtime/src/execute/compile-plan.ts b/packages/runtime/src/execute/compile-plan.ts index ca7c6b2cc..61d8d2db3 100644 --- a/packages/runtime/src/execute/compile-plan.ts +++ b/packages/runtime/src/execute/compile-plan.ts @@ -131,10 +131,13 @@ export default (plan: ExecutionPlan) => { if (job.linker) { newStep.linker = job.linker; - } else if (job.adaptor) { + } else if (job.adaptors) { const job = step as Job; - const { name, version } = getNameAndVersion(job.adaptor!); - newStep.linker = { [name]: { version: version! } }; + newStep.linker ??= {}; + for (const adaptor of job.adaptors!) { + const { name, version } = getNameAndVersion(adaptor); + newStep.linker[name] = { version: version! }; + } } if (step.next) { diff --git a/packages/runtime/src/util/validate-plan.ts b/packages/runtime/src/util/validate-plan.ts index f43eeb2e1..fd1541b9e 100644 --- a/packages/runtime/src/util/validate-plan.ts +++ b/packages/runtime/src/util/validate-plan.ts @@ -70,10 +70,12 @@ export const buildModel = ({ workflow }: ExecutionPlan) => { }; const assertStart = (plan: ExecutionPlan) => { - const { start } = plan.options; - if (typeof start === 'string') { - if (!plan.workflow.steps.find(({ id }) => id === start)) { - throw new ValidationError(`Could not find start job: ${start}`); + if (plan.options) { + const { start } = plan.options; + if (typeof start === 'string') { + if (!plan.workflow.steps.find(({ id }) => id === start)) { + throw new ValidationError(`Could not find start job: ${start}`); + } } } }; diff --git a/packages/runtime/test/execute/compile-plan.test.ts b/packages/runtime/test/execute/compile-plan.test.ts index 809c2a664..8fc57bd14 100644 --- a/packages/runtime/test/execute/compile-plan.test.ts +++ b/packages/runtime/test/execute/compile-plan.test.ts @@ -208,12 +208,12 @@ test('should write adaptor versions', (t) => { { id: 'x', expression: '.', - adaptor: 'x@1.0', + adaptors: ['x@1.0'], }, { id: 'y', expression: '.', - adaptor: 'y@1.0', + adaptors: ['y@1.0'], }, ], }, diff --git a/packages/runtime/test/runtime.test.ts b/packages/runtime/test/runtime.test.ts index 5c9c65ce6..56b8ad478 100644 --- a/packages/runtime/test/runtime.test.ts +++ b/packages/runtime/test/runtime.test.ts @@ -777,21 +777,21 @@ test('run a workflow using the repo using a specific version', async (t) => { }); test('run a workflow using the repo with multiple versions of the same adaptor', async (t) => { - const plan = { + const plan: ExecutionPlan = { workflow: { steps: [ { id: 'a', expression: `import result from 'ultimate-answer'; export default [(s) => { s.data.a = result; return s;}];`, - adaptor: 'ultimate-answer@1.0.0', + adaptors: ['ultimate-answer@1.0.0'], next: { b: true }, }, { id: 'b', expression: `import result from 'ultimate-answer'; export default [(s) => { s.data.b = result; return s;}];`, - adaptor: 'ultimate-answer@2.0.0', + adaptors: ['ultimate-answer@2.0.0'], }, ], }, From 1c57b7e32b78d69cee153609a211604178efdd30 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Mon, 21 Oct 2024 15:06:20 +0100 Subject: [PATCH 10/52] cli:type tweak --- packages/cli/src/execute/handler.ts | 8 ++++---- packages/cli/src/types.ts | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/cli/src/execute/handler.ts b/packages/cli/src/execute/handler.ts index 892d8f9cc..294e3c1c9 100644 --- a/packages/cli/src/execute/handler.ts +++ b/packages/cli/src/execute/handler.ts @@ -85,7 +85,7 @@ const executeHandler = async (options: ExecuteOptions, logger: Logger) => { if (options.start) { customStart = matchStep( plan, - options.start ?? plan.options.start, + options.start ?? plan.options!.start, 'start', logger ); @@ -95,7 +95,7 @@ const executeHandler = async (options: ExecuteOptions, logger: Logger) => { if (options.end) { customEnd = matchStep( plan, - options.end ?? plan.options.end, + options.end ?? plan.options!.end, 'end', logger ); @@ -113,8 +113,8 @@ const executeHandler = async (options: ExecuteOptions, logger: Logger) => { const finalPlan = { ...plan, options: { - ...plan.options, - start: customStart || plan.options.start, + ...plan.options!, + start: customStart || plan.options!.start, end: customEnd, }, workflow: plan.workflow, diff --git a/packages/cli/src/types.ts b/packages/cli/src/types.ts index 1ca6ad63c..ba0b3b8cf 100644 --- a/packages/cli/src/types.ts +++ b/packages/cli/src/types.ts @@ -13,7 +13,7 @@ export type OldCLIWorkflow = { export type CLIExecutionPlan = { id?: string; - options: WorkflowOptions; + options?: WorkflowOptions; workflow: { id?: UUID; name?: string; From 1714d784f3a6d82b3a3fbf71a86f277cf42a8090 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Mon, 21 Oct 2024 15:09:06 +0100 Subject: [PATCH 11/52] compiler: update tests --- packages/compiler/test/compile.test.ts | 50 +++++++++++++++----------- 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/packages/compiler/test/compile.test.ts b/packages/compiler/test/compile.test.ts index 9eca1efea..abd4d220c 100644 --- a/packages/compiler/test/compile.test.ts +++ b/packages/compiler/test/compile.test.ts @@ -56,10 +56,12 @@ test('compile multiple operations', (t) => { test('add imports', (t) => { const options = { 'add-imports': { - adaptor: { - name: '@openfn/language-common', - exports: ['fn'], - }, + adaptors: [ + { + name: '@openfn/language-common', + exports: ['fn'], + }, + ], }, }; const source = 'fn();'; @@ -71,10 +73,12 @@ test('add imports', (t) => { test('do not add imports', (t) => { const options = { 'add-imports': { - adaptor: { - name: '@openfn/language-common', - exports: ['fn'], - }, + adaptors: [ + { + name: '@openfn/language-common', + exports: ['fn'], + }, + ], }, }; // This example already has the correct imports declared, so add-imports should do nothing @@ -87,9 +91,11 @@ test('do not add imports', (t) => { test('dumbly add imports', (t) => { const options = { 'add-imports': { - adaptor: { - name: '@openfn/language-common', - }, + adaptors: [ + { + name: '@openfn/language-common', + }, + ], }, }; // This example already has the correct imports declared, so add-imports should do nothing @@ -102,11 +108,13 @@ test('dumbly add imports', (t) => { test('add imports with export all', (t) => { const options = { 'add-imports': { - adaptor: { - name: '@openfn/language-common', - exports: ['fn'], - exportAll: true, - }, + adaptors: [ + { + name: '@openfn/language-common', + exports: ['fn'], + exportAll: true, + }, + ], }, }; const source = 'fn();'; @@ -154,10 +162,12 @@ test('compile a lazy state ($) expression', (t) => { test('compile a lazy state ($) expression with dumb imports', (t) => { const options = { 'add-imports': { - adaptor: { - name: '@openfn/language-common', - exportAll: true, - }, + adaptors: [ + { + name: '@openfn/language-common', + exportAll: true, + }, + ], }, }; const source = 'get($.data.endpoint);'; From 8305849f8e211cc52ef32419d45a0adfdc9c32bb Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Mon, 21 Oct 2024 15:14:15 +0100 Subject: [PATCH 12/52] fix type --- packages/lightning-mock/test/server.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/lightning-mock/test/server.test.ts b/packages/lightning-mock/test/server.test.ts index 5b0e9a3c1..6e9d48945 100644 --- a/packages/lightning-mock/test/server.test.ts +++ b/packages/lightning-mock/test/server.test.ts @@ -36,6 +36,7 @@ test.serial('should setup an run at /POST /run', async (t) => { user: 'john', password: 'rambo', }, + adaptor: 'abc', }, ], edges: [], From a71ef462e2c812ddd85062b109cd538443004e2a Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Mon, 21 Oct 2024 15:15:14 +0100 Subject: [PATCH 13/52] engine: fix test --- packages/engine-multi/test/errors.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/engine-multi/test/errors.test.ts b/packages/engine-multi/test/errors.test.ts index 8ba54cd20..38d0979b1 100644 --- a/packages/engine-multi/test/errors.test.ts +++ b/packages/engine-multi/test/errors.test.ts @@ -207,14 +207,14 @@ test.serial('after uncaught exception, free up the pool', (t) => { }); }); -test.serial('emit a crash error on process.exit()', (t) => { +test.serial.only('emit a crash error on process.exit()', (t) => { return new Promise((done) => { const plan = { id: 'z', workflow: { steps: [ { - adaptor: '@openfn/helper@1.0.0', + adaptors: ['@openfn/helper@1.0.0'], expression: 'export default [exit()]', }, ], From 8821bc15367084780e12e463ee9b8c21071f8922 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Mon, 21 Oct 2024 15:16:49 +0100 Subject: [PATCH 14/52] tests: fix execute tests --- integration-tests/execute/src/execute.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/integration-tests/execute/src/execute.ts b/integration-tests/execute/src/execute.ts index 49aba31d7..b4fd40f12 100644 --- a/integration-tests/execute/src/execute.ts +++ b/integration-tests/execute/src/execute.ts @@ -6,10 +6,12 @@ const execute = async (job: string, state: any, adaptor = 'common') => { // compile with common and dumb imports const options = { 'add-imports': { - adaptor: { - name: `@openfn/language-${adaptor}`, - exportAll: true, - }, + adaptors: [ + { + name: `@openfn/language-${adaptor}`, + exportAll: true, + }, + ], }, }; const compiled = compiler(job, options); From ba42d569e9433dbe9a3b205a6bb4d2cea472a444 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Mon, 21 Oct 2024 16:09:38 +0100 Subject: [PATCH 15/52] engine: fix another test --- packages/engine-multi/test/integration.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/engine-multi/test/integration.test.ts b/packages/engine-multi/test/integration.test.ts index 28dc86259..bdaf48fb9 100644 --- a/packages/engine-multi/test/integration.test.ts +++ b/packages/engine-multi/test/integration.test.ts @@ -214,7 +214,7 @@ test.serial('trigger workflow-log for adaptor logs', (t) => { // This will trigger console.log from inside the adaptor // rather than from job code directly expression: "log('hola')", - adaptor: '@openfn/helper@1.0.0', + adaptors: ['@openfn/helper@1.0.0'], }, ]); From ad1560830f384a3d690f0c556c1df3926c7e9c0e Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Mon, 21 Oct 2024 16:11:39 +0100 Subject: [PATCH 16/52] worker: more typings --- packages/ws-worker/src/util/create-run-state.ts | 4 ++-- packages/ws-worker/test/util/create-run-state.test.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/ws-worker/src/util/create-run-state.ts b/packages/ws-worker/src/util/create-run-state.ts index bb30fa05a..deaa76aad 100644 --- a/packages/ws-worker/src/util/create-run-state.ts +++ b/packages/ws-worker/src/util/create-run-state.ts @@ -22,8 +22,8 @@ export default (plan: ExecutionPlan, input?: Lazy): RunState => { // find the first job const jobs = plan.workflow.steps as Job[]; let startNode = jobs[0]; - if (plan.options.start) { - startNode = jobs.find(({ id }) => id === plan.options.start)!; + if (plan.options?.start) { + startNode = jobs.find(({ id }) => id === plan.options?.start)!; } const initialRuns: string[] = []; diff --git a/packages/ws-worker/test/util/create-run-state.test.ts b/packages/ws-worker/test/util/create-run-state.test.ts index 31ad59577..8df4f5cf8 100644 --- a/packages/ws-worker/test/util/create-run-state.test.ts +++ b/packages/ws-worker/test/util/create-run-state.test.ts @@ -9,7 +9,7 @@ const createPlan = (jobs: Partial[]) => steps: jobs.map((j) => ({ expression: '.', ...j })), }, options: {}, - } as ExecutionPlan); + } as Required); test('create run', (t) => { const plan = createPlan([{ id: 'a' }]); From af974896283bd99f235a29436457c41bd9782c54 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Tue, 22 Oct 2024 10:52:06 +0100 Subject: [PATCH 17/52] tidyup monorepo in preloadAdaptorExports --- packages/cli/src/compile/compile.ts | 6 +----- packages/compiler/src/util.ts | 1 - packages/engine-multi/src/api/compile.ts | 2 +- 3 files changed, 2 insertions(+), 7 deletions(-) diff --git a/packages/cli/src/compile/compile.ts b/packages/cli/src/compile/compile.ts index a2f8285f6..cbe640340 100644 --- a/packages/cli/src/compile/compile.ts +++ b/packages/cli/src/compile/compile.ts @@ -124,11 +124,7 @@ export const loadTransformOptions = async ( const path = await resolveSpecifierPath(pattern, opts.repoDir, log); if (path) { try { - exports = await preloadAdaptorExports( - path, - opts.useAdaptorsMonorepo, - log - ); + exports = await preloadAdaptorExports(path, log); } catch (e) { log.error(`Failed to load adaptor typedefs from path ${path}`); log.error(e); diff --git a/packages/compiler/src/util.ts b/packages/compiler/src/util.ts index fa197079e..ee0b0c92d 100644 --- a/packages/compiler/src/util.ts +++ b/packages/compiler/src/util.ts @@ -24,7 +24,6 @@ export const isRelativeSpecifier = (specifier: string) => // But we may relax this later. export const preloadAdaptorExports = async ( pathToModule: string, - _useMonorepo?: boolean, log?: Logger ) => { const project = new Project(); diff --git a/packages/engine-multi/src/api/compile.ts b/packages/engine-multi/src/api/compile.ts index c47660adf..07061e8e0 100644 --- a/packages/engine-multi/src/api/compile.ts +++ b/packages/engine-multi/src/api/compile.ts @@ -56,7 +56,7 @@ const compileJob = async ( if (adaptor && repoDir) { // TODO I probably dont want to log this stuff const pathToAdaptor = await getModulePath(adaptor, repoDir, logger); - const exports = await preloadAdaptorExports(pathToAdaptor!, false, logger); + const exports = await preloadAdaptorExports(pathToAdaptor!, logger); compilerOptions['add-imports'] = { adaptor: { name: stripVersionSpecifier(adaptor), From 1c79dc1b7259ecd4fd40bdbe4ad6171234aa4160 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Tue, 22 Oct 2024 11:27:25 +0100 Subject: [PATCH 18/52] worker: automatically append the collections adaptor to steps that need it --- .changeset/loud-crabs-walk.md | 5 +++++ .../src/util/convert-lightning-plan.ts | 14 +++++++++++++ .../test/util/convert-lightning-plan.test.ts | 21 +++++++++++++++++++ 3 files changed, 40 insertions(+) create mode 100644 .changeset/loud-crabs-walk.md diff --git a/.changeset/loud-crabs-walk.md b/.changeset/loud-crabs-walk.md new file mode 100644 index 000000000..96c3ddfc5 --- /dev/null +++ b/.changeset/loud-crabs-walk.md @@ -0,0 +1,5 @@ +--- +'@openfn/ws-worker': patch +--- + +Append the collections adaptor to steps that need it diff --git a/packages/ws-worker/src/util/convert-lightning-plan.ts b/packages/ws-worker/src/util/convert-lightning-plan.ts index eff47d12a..49e090276 100644 --- a/packages/ws-worker/src/util/convert-lightning-plan.ts +++ b/packages/ws-worker/src/util/convert-lightning-plan.ts @@ -39,6 +39,18 @@ const mapTriggerEdgeCondition = (edge: LightningEdge) => { return condition; }; +// This function will look at every step and decide whether the collections adaptor +// should be added to the array +const appendCollectionsAdaptor = (plan: ExecutionPlan) => { + plan.workflow.steps.forEach((step) => { + const job = step as Job; + if (job.expression?.match(/(collections\.)/)) { + job.adaptors ??= []; + job.adaptors.push('@openfn/language-collections'); // what about version? Is this safe? + } + }); +}; + // Options which relate to this execution but are not part of the plan export type WorkerRunOptions = ExecuteOptions & { // Defaults to true - must be explicity false to stop dataclips being sent @@ -170,6 +182,8 @@ export default ( plan.workflow.name = run.name; } + appendCollectionsAdaptor(plan as ExecutionPlan); + return { plan: plan as ExecutionPlan, options: engineOpts, diff --git a/packages/ws-worker/test/util/convert-lightning-plan.test.ts b/packages/ws-worker/test/util/convert-lightning-plan.test.ts index 79148ea2d..e7922c1eb 100644 --- a/packages/ws-worker/test/util/convert-lightning-plan.test.ts +++ b/packages/ws-worker/test/util/convert-lightning-plan.test.ts @@ -588,3 +588,24 @@ test('convert edge condition always', (t) => { const edge = job.next as Record; t.false(edge.b.hasOwnProperty('condition')); }); + +test('append the collections adaptor to jobs that use it', (t) => { + const run: Partial = { + id: 'w', + jobs: [ + createNode({ id: 'a' }), + createNode({ + id: 'b', + body: 'collections.each("c", "k", (state) => state)', + }), + ], + triggers: [{ id: 't', type: 'cron' }], + edges: [createEdge('a', 'b')], + }; + const { plan } = convertPlan(run as LightningPlan); + + const [_t, a, b] = plan.workflow.steps; + + t.deepEqual(a.adaptors, ['common']); + t.deepEqual(b.adaptors, ['common', '@openfn/language-collections']); +}); From 3463ff95d96facd99dcafa0da76aa929c615bf57 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Tue, 22 Oct 2024 15:11:25 +0100 Subject: [PATCH 19/52] runtime: support global congfiguration on state assembly --- .changeset/healthy-laws-sniff.md | 5 ++ packages/runtime/src/util/assemble-state.ts | 9 ++-- .../runtime/test/util/assemble-state.test.ts | 49 ++++++++++++++++++- 3 files changed, 55 insertions(+), 8 deletions(-) create mode 100644 .changeset/healthy-laws-sniff.md diff --git a/.changeset/healthy-laws-sniff.md b/.changeset/healthy-laws-sniff.md new file mode 100644 index 000000000..70905b01e --- /dev/null +++ b/.changeset/healthy-laws-sniff.md @@ -0,0 +1,5 @@ +--- +'@openfn/runtime': major +--- + +Support global credential object on a workflow diff --git a/packages/runtime/src/util/assemble-state.ts b/packages/runtime/src/util/assemble-state.ts index 84f5fc12e..e25b94eeb 100644 --- a/packages/runtime/src/util/assemble-state.ts +++ b/packages/runtime/src/util/assemble-state.ts @@ -13,7 +13,8 @@ const assembleData = (initialData: any, defaultData = {}) => { const assembleState = ( initialState: any = {}, // previous or initial state configuration = {}, - defaultState: any = {} // This is default state provided by the job + defaultState: any = {}, // This is default state provided by the job + globalCredentials: any = {} ) => { const obj = { ...defaultState, @@ -29,11 +30,7 @@ const assembleState = ( } Object.assign(obj, { - configuration: Object.assign( - {}, - initialState.configuration ?? {}, - configuration - ), + configuration: Object.assign({}, globalCredentials, configuration), data: assembleData(initialState.data, defaultState.data), }); diff --git a/packages/runtime/test/util/assemble-state.test.ts b/packages/runtime/test/util/assemble-state.test.ts index eac478b93..b27c99c13 100644 --- a/packages/runtime/test/util/assemble-state.test.ts +++ b/packages/runtime/test/util/assemble-state.test.ts @@ -67,7 +67,7 @@ test('Initial data does not have to be an object', (t) => { }); }); -test('merges default and initial config objects', (t) => { +test('does not merge default and initial config objects', (t) => { const initial = { configuration: { x: 1 } }; const defaultState = undefined; const config = { y: 1 }; @@ -75,7 +75,6 @@ test('merges default and initial config objects', (t) => { const result = assembleState(initial, config, defaultState); t.deepEqual(result, { configuration: { - x: 1, y: 1, }, data: {}, @@ -95,3 +94,49 @@ test('configuration overrides initialState.configuration', (t) => { data: {}, }); }); + +test('global credentials should be added', (t) => { + const initial = {}; + const defaultState = undefined; + const config = undefined; + const global = { collection_token: 'j.w.t' }; + + const result = assembleState(initial, config, defaultState, global); + t.deepEqual(result, { + configuration: { + collection_token: 'j.w.t', + }, + data: {}, + }); +}); + +test('global credentials should be merged in', (t) => { + const initial = { configuration: { x: 1 } }; + const defaultState = undefined; + const config = { x: 2 }; + const global = { collection_token: 'j.w.t' }; + + const result = assembleState(initial, config, defaultState, global); + t.deepEqual(result, { + configuration: { + x: 2, + collection_token: 'j.w.t', + }, + data: {}, + }); +}); + +test('local credentials should override global credentials', (t) => { + const initial = { configuration: { x: 1 } }; + const defaultState = undefined; + const config = { collection_token: 'x.y.z' }; + const global = { collection_token: 'j.w.t' }; + + const result = assembleState(initial, config, defaultState, global); + t.deepEqual(result, { + configuration: { + collection_token: 'x.y.z', + }, + data: {}, + }); +}); From c6d8ac957897c0a5db9b9a6924f0b68357e0be8e Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Tue, 22 Oct 2024 16:39:02 +0100 Subject: [PATCH 20/52] runtime: support global credentials --- packages/lexicon/core.d.ts | 4 ++ packages/runtime/src/execute/compile-plan.ts | 3 ++ packages/runtime/src/execute/step.ts | 12 +++-- packages/runtime/src/types.ts | 1 + .../runtime/test/execute/compile-plan.test.ts | 14 +++++ packages/runtime/test/runtime.test.ts | 54 ++++++++++++------- 6 files changed, 64 insertions(+), 24 deletions(-) diff --git a/packages/lexicon/core.d.ts b/packages/lexicon/core.d.ts index f5626a546..dee4a491d 100644 --- a/packages/lexicon/core.d.ts +++ b/packages/lexicon/core.d.ts @@ -23,6 +23,10 @@ export type Workflow = { name?: string; steps: Array; + + // global credentials + // (gets applied to every configuration object) + credentials?: Record; }; /** diff --git a/packages/runtime/src/execute/compile-plan.ts b/packages/runtime/src/execute/compile-plan.ts index 61d8d2db3..65a3b6e52 100644 --- a/packages/runtime/src/execute/compile-plan.ts +++ b/packages/runtime/src/execute/compile-plan.ts @@ -106,6 +106,9 @@ export default (plan: ExecutionPlan) => { start: options.start ?? workflow.steps[0]?.id!, }, }; + if (workflow.credentials) { + newPlan.workflow.credentials = workflow.credentials; + } const maybeAssign = (a: any, b: any, keys: Array) => { keys.forEach((key) => { diff --git a/packages/runtime/src/execute/step.ts b/packages/runtime/src/execute/step.ts index de9d90112..7e90d0ed5 100644 --- a/packages/runtime/src/execute/step.ts +++ b/packages/runtime/src/execute/step.ts @@ -80,7 +80,7 @@ const executeStep = async ( step: CompiledStep, input: State = {} ): Promise<{ next: StepId[]; state: any }> => { - const { opts, notify, logger, report } = ctx; + const { opts, notify, logger, report, plan } = ctx; const duration = Date.now(); @@ -104,12 +104,16 @@ const executeStep = async ( opts.callbacks?.resolveCredential! // cheat - we need to handle the error case here ); - const globals = await loadState( + const globalState = await loadState( job, opts.callbacks?.resolveState! // and here ); - - const state = assembleState(clone(input), configuration, globals); + const state = assembleState( + clone(input), + configuration, + globalState, + plan.workflow.credentials + ); notify(NOTIFY_INIT_COMPLETE, { jobId, diff --git a/packages/runtime/src/types.ts b/packages/runtime/src/types.ts index 41fc80b72..c2756184e 100644 --- a/packages/runtime/src/types.ts +++ b/packages/runtime/src/types.ts @@ -36,6 +36,7 @@ export type Lazy = string | T; export type CompiledExecutionPlan = { workflow: { steps: Record; + credentials?: Record; }; options: WorkflowOptions & { start: StepId; diff --git a/packages/runtime/test/execute/compile-plan.test.ts b/packages/runtime/test/execute/compile-plan.test.ts index 8fc57bd14..922b593ea 100644 --- a/packages/runtime/test/execute/compile-plan.test.ts +++ b/packages/runtime/test/execute/compile-plan.test.ts @@ -33,6 +33,20 @@ const planWithEdge = (edge: Partial) => ({ }, }); +test('should preserve global credentials ', (t) => { + const compiledPlan = compilePlan({ + id: 'a', + workflow: { + steps: [{ id: 'a', expression: 'a' }], + credentials: { collections_token: 'j.w.t.' }, + }, + }); + + t.deepEqual(compiledPlan.workflow.credentials, { + collections_token: 'j.w.t.', + }); +}); + test('should preserve the start option', (t) => { const compiledPlan = compilePlan({ id: 'a', diff --git a/packages/runtime/test/runtime.test.ts b/packages/runtime/test/runtime.test.ts index 56b8ad478..a55c5190d 100644 --- a/packages/runtime/test/runtime.test.ts +++ b/packages/runtime/test/runtime.test.ts @@ -12,8 +12,6 @@ import { } from '../src'; import run from '../src/runtime'; -type ExecutionPlanNoOptions = Omit; - test('run simple expression', async (t) => { const expression = 'export default [(s) => {s.data.done = true; return s}]'; @@ -22,7 +20,7 @@ test('run simple expression', async (t) => { }); test('run a simple workflow', async (t) => { - const plan: ExecutionPlanNoOptions = { + const plan: ExecutionPlan = { workflow: { steps: [ { expression: 'export default [(s) => ({ data: { done: true } })]' }, @@ -34,6 +32,22 @@ test('run a simple workflow', async (t) => { t.true(result.data.done); }); +test('run a workflow with global config', async (t) => { + const plan: ExecutionPlan = { + workflow: { + steps: [{ expression: 'export default [(s) => state.configuration];' }], + credentials: { + collection_token: 'j.w.t', + }, + }, + }; + + const result: any = await run(plan); + t.deepEqual(result, { + collection_token: 'j.w.t', + }); +}); + test('run a workflow and notify major events', async (t) => { const counts: Record = {}; const notify = (name: string) => { @@ -47,7 +61,7 @@ test('run a workflow and notify major events', async (t) => { notify, }; - const plan: ExecutionPlanNoOptions = { + const plan: ExecutionPlan = { workflow: { steps: [{ expression: 'export default [(s) => s]' }], }, @@ -76,7 +90,7 @@ test('notify job error even after fail', async (t) => { notify, }; - const plan: ExecutionPlanNoOptions = { + const plan: ExecutionPlan = { workflow: { steps: [ { id: 'a', expression: 'export default [(s) => s.data.x = s.err.z ]' }, @@ -102,7 +116,7 @@ test('notify job error even after crash', async (t) => { notify, }; - const plan: ExecutionPlanNoOptions = { + const plan: ExecutionPlan = { workflow: { steps: [{ id: 'a', expression: 'export default [() => s]' }] }, }; @@ -139,7 +153,7 @@ test('resolve a credential', async (t) => { }); test('resolve initial state', async (t) => { - const plan: ExecutionPlanNoOptions = { + const plan: ExecutionPlan = { workflow: { steps: [ { @@ -174,7 +188,7 @@ test('run a workflow with two jobs and call callbacks', async (t) => { notify, }; - const plan: ExecutionPlanNoOptions = { + const plan: ExecutionPlan = { workflow: { steps: [ { id: 'a', expression: 'export default [(s) => s]', next: { b: true } }, @@ -192,7 +206,7 @@ test('run a workflow with two jobs and call callbacks', async (t) => { }); test('run a workflow with state and parallel branching', async (t) => { - const plan: ExecutionPlanNoOptions = { + const plan: ExecutionPlan = { workflow: { steps: [ { @@ -239,7 +253,7 @@ test('run a workflow with state and parallel branching', async (t) => { }); test('run a workflow with a leaf step called multiple times', async (t) => { - const plan: ExecutionPlanNoOptions = { + const plan: ExecutionPlan = { workflow: { steps: [ { @@ -294,7 +308,7 @@ test('run a workflow with a leaf step called multiple times', async (t) => { // TODO this test sort of shows why input state on the plan object is a bit funky // running the same plan with two inputs is pretty clunky test('run a workflow with state and conditional branching', async (t) => { - const plan: ExecutionPlanNoOptions = { + const plan: ExecutionPlan = { workflow: { steps: [ { @@ -422,7 +436,7 @@ test('run a workflow with a start and end', async (t) => { }); test('run a workflow with a trigger node', async (t) => { - const plan: ExecutionPlanNoOptions = { + const plan: ExecutionPlan = { workflow: { steps: [ { @@ -442,7 +456,7 @@ test('run a workflow with a trigger node', async (t) => { }); test('prefer initial state to inline state', async (t) => { - const plan: ExecutionPlanNoOptions = { + const plan: ExecutionPlan = { workflow: { steps: [ { @@ -464,7 +478,7 @@ test('prefer initial state to inline state', async (t) => { }); test('Allow a job to return undefined', async (t) => { - const plan: ExecutionPlanNoOptions = { + const plan: ExecutionPlan = { workflow: { steps: [{ expression: 'export default [() => {}]' }], }, @@ -475,7 +489,7 @@ test('Allow a job to return undefined', async (t) => { }); test('log errors, write to state, and continue', async (t) => { - const plan: ExecutionPlanNoOptions = { + const plan: ExecutionPlan = { workflow: { steps: [ { @@ -507,7 +521,7 @@ test('log errors, write to state, and continue', async (t) => { }); test('log job code to the job logger', async (t) => { - const plan: ExecutionPlanNoOptions = { + const plan: ExecutionPlan = { workflow: { steps: [ { @@ -529,7 +543,7 @@ test('log job code to the job logger', async (t) => { }); test('log and serialize an error to the job logger', async (t) => { - const plan: ExecutionPlanNoOptions = { + const plan: ExecutionPlan = { workflow: { steps: [ { @@ -556,7 +570,7 @@ test('log and serialize an error to the job logger', async (t) => { }); test('error reports can be overwritten', async (t) => { - const plan: ExecutionPlanNoOptions = { + const plan: ExecutionPlan = { workflow: { steps: [ { @@ -580,7 +594,7 @@ test('error reports can be overwritten', async (t) => { // This tracks current behaviour but I don't know if it's right test('stuff written to state before an error is preserved', async (t) => { - const plan: ExecutionPlanNoOptions = { + const plan: ExecutionPlan = { workflow: { steps: [ { @@ -609,7 +623,7 @@ test('data can be an array (expression)', async (t) => { }); test('data can be an array (workflow)', async (t) => { - const plan: ExecutionPlanNoOptions = { + const plan: ExecutionPlan = { workflow: { steps: [ { From fd0e499aa0328eed5098b8d8fbc740baa428cc64 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Tue, 22 Oct 2024 17:10:17 +0100 Subject: [PATCH 21/52] worker: create global credential for collections --- .changeset/eleven-cougars-exist.md | 5 ++++ packages/ws-worker/CHANGELOG.md | 11 +++++++++ packages/ws-worker/package.json | 2 +- packages/ws-worker/src/server.ts | 9 ++++++++ .../src/util/convert-lightning-plan.ts | 11 ++++++++- packages/ws-worker/test/lightning.test.ts | 23 +++++++++++++++++++ .../test/util/convert-lightning-plan.test.ts | 23 +++++++++++++++++++ 7 files changed, 82 insertions(+), 2 deletions(-) create mode 100644 .changeset/eleven-cougars-exist.md diff --git a/.changeset/eleven-cougars-exist.md b/.changeset/eleven-cougars-exist.md new file mode 100644 index 000000000..53e2817d7 --- /dev/null +++ b/.changeset/eleven-cougars-exist.md @@ -0,0 +1,5 @@ +--- +'@openfn/ws-worker': minor +--- + +Support collections diff --git a/packages/ws-worker/CHANGELOG.md b/packages/ws-worker/CHANGELOG.md index cf78dec30..12f3005bd 100644 --- a/packages/ws-worker/CHANGELOG.md +++ b/packages/ws-worker/CHANGELOG.md @@ -1,5 +1,16 @@ # ws-worker +## 1.7.1 + +### Patch Changes + +- 1c79dc1: Append the collections adaptor to steps that need it +- b15f151: Update worker to use adaptors as an array on xplans. Internal only change. +- Updated dependencies [3463ff9] +- Updated dependencies [7245bf7] + - @openfn/runtime@2.0.0 + - @openfn/engine-multi@1.4.0 + ## 1.7.0 ### Minor Changes diff --git a/packages/ws-worker/package.json b/packages/ws-worker/package.json index c10dac7ce..cd0c2a8f3 100644 --- a/packages/ws-worker/package.json +++ b/packages/ws-worker/package.json @@ -1,6 +1,6 @@ { "name": "@openfn/ws-worker", - "version": "1.7.0", + "version": "1.7.1", "description": "A Websocket Worker to connect Lightning to a Runtime Engine", "main": "dist/index.js", "type": "module", diff --git a/packages/ws-worker/src/server.ts b/packages/ws-worker/src/server.ts index c77fb755c..d9562510f 100644 --- a/packages/ws-worker/src/server.ts +++ b/packages/ws-worker/src/server.ts @@ -202,6 +202,15 @@ function createServer(engine: RuntimeEngine, options: ServerOptions = {}) { input, } = await joinRunChannel(app.socket, token, id, logger); + // Setup collections + if (plan.workflow.credentials?.collections_token) { + plan.workflow.credentials.collections_token = token; + } + if (plan.workflow.credentials?.collections_endpoint) { + plan.workflow.credentials.collections_endpoint = + app.options.lightning; + } + // Default the payload limit if it's not otherwise set on the run options if (!('payloadLimitMb' in options)) { options.payloadLimitMb = app.options.payloadLimitMb; diff --git a/packages/ws-worker/src/util/convert-lightning-plan.ts b/packages/ws-worker/src/util/convert-lightning-plan.ts index 49e090276..01eeddcac 100644 --- a/packages/ws-worker/src/util/convert-lightning-plan.ts +++ b/packages/ws-worker/src/util/convert-lightning-plan.ts @@ -42,13 +42,16 @@ const mapTriggerEdgeCondition = (edge: LightningEdge) => { // This function will look at every step and decide whether the collections adaptor // should be added to the array const appendCollectionsAdaptor = (plan: ExecutionPlan) => { + let hasCollections; plan.workflow.steps.forEach((step) => { const job = step as Job; if (job.expression?.match(/(collections\.)/)) { + hasCollections = true; job.adaptors ??= []; job.adaptors.push('@openfn/language-collections'); // what about version? Is this safe? } }); + return hasCollections; }; // Options which relate to this execution but are not part of the plan @@ -182,7 +185,13 @@ export default ( plan.workflow.name = run.name; } - appendCollectionsAdaptor(plan as ExecutionPlan); + const hasCollections = appendCollectionsAdaptor(plan as ExecutionPlan); + if (hasCollections) { + plan.workflow.credentials = { + collections_token: true, + collections_endpoint: 'https://app.openfn.org', + }; + } return { plan: plan as ExecutionPlan, diff --git a/packages/ws-worker/test/lightning.test.ts b/packages/ws-worker/test/lightning.test.ts index 4d9b08924..4b508bcce 100644 --- a/packages/ws-worker/test/lightning.test.ts +++ b/packages/ws-worker/test/lightning.test.ts @@ -258,6 +258,29 @@ test.serial('should run a run which returns initial state', async (t) => { }); }); +test.serial('should run a run with the collections adaptor', async (t) => { + return new Promise((done) => { + const run = { + id: 'run-1', + jobs: [ + { + // This should be enough to fake the worker into + // loading the collections machinery + body: 'fn((s) => /* collections.get */ s.configuration)', + }, + ], + }; + + lng.waitForResult(run.id).then((result: any) => { + t.is(result.collections_endpoint, urls.lng); + t.is(typeof result.collections_token, 'string'); + done(); + }); + + lng.enqueueRun(run); + }); +}); + // A basic high level integration test to ensure the whole loop works // This checks the events received by the lightning websocket test.serial( diff --git a/packages/ws-worker/test/util/convert-lightning-plan.test.ts b/packages/ws-worker/test/util/convert-lightning-plan.test.ts index e7922c1eb..9f55f6a57 100644 --- a/packages/ws-worker/test/util/convert-lightning-plan.test.ts +++ b/packages/ws-worker/test/util/convert-lightning-plan.test.ts @@ -609,3 +609,26 @@ test('append the collections adaptor to jobs that use it', (t) => { t.deepEqual(a.adaptors, ['common']); t.deepEqual(b.adaptors, ['common', '@openfn/language-collections']); }); + +test('append the collections credential to jobs that use it', (t) => { + const run: Partial = { + id: 'w', + jobs: [ + createNode({ id: 'a' }), + createNode({ + id: 'b', + body: 'collections.each("c", "k", (state) => state)', + }), + ], + triggers: [{ id: 't', type: 'cron' }], + edges: [createEdge('a', 'b')], + }; + const { plan } = convertPlan(run as LightningPlan); + + const creds = plan.workflow.credentials; + + t.deepEqual(creds, { + collections_token: true, + collections_endpoint: 'https://app.openfn.org', + }); +}); From 4e659a8cf54ec83261354c45babc1fdb0c1ffc51 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Tue, 22 Oct 2024 17:29:27 +0100 Subject: [PATCH 22/52] fixse --- packages/runtime/src/execute/step.ts | 2 +- packages/ws-worker/test/util/convert-lightning-plan.test.ts | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/runtime/src/execute/step.ts b/packages/runtime/src/execute/step.ts index 7e90d0ed5..2ee45155e 100644 --- a/packages/runtime/src/execute/step.ts +++ b/packages/runtime/src/execute/step.ts @@ -112,7 +112,7 @@ const executeStep = async ( clone(input), configuration, globalState, - plan.workflow.credentials + plan.workflow?.credentials ); notify(NOTIFY_INIT_COMPLETE, { diff --git a/packages/ws-worker/test/util/convert-lightning-plan.test.ts b/packages/ws-worker/test/util/convert-lightning-plan.test.ts index 9f55f6a57..578fe7ebd 100644 --- a/packages/ws-worker/test/util/convert-lightning-plan.test.ts +++ b/packages/ws-worker/test/util/convert-lightning-plan.test.ts @@ -606,7 +606,9 @@ test('append the collections adaptor to jobs that use it', (t) => { const [_t, a, b] = plan.workflow.steps; + // @ts-ignore t.deepEqual(a.adaptors, ['common']); + // @ts-ignore t.deepEqual(b.adaptors, ['common', '@openfn/language-collections']); }); From 560fce77e85ea2bf1f1fb10c240cf93be4cbabc8 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Wed, 23 Oct 2024 11:21:59 +0100 Subject: [PATCH 23/52] worker: accept collections version from CLI and refactor some stuff --- packages/ws-worker/src/channels/run.ts | 18 ++------ packages/ws-worker/src/server.ts | 44 +++++++++++++++---- packages/ws-worker/src/start.ts | 1 + packages/ws-worker/src/util/cli.ts | 11 +++++ .../src/util/convert-lightning-plan.ts | 15 +++++-- 5 files changed, 62 insertions(+), 27 deletions(-) diff --git a/packages/ws-worker/src/channels/run.ts b/packages/ws-worker/src/channels/run.ts index 9cde5c97a..a8f925084 100644 --- a/packages/ws-worker/src/channels/run.ts +++ b/packages/ws-worker/src/channels/run.ts @@ -1,9 +1,7 @@ -import type { ExecutionPlan, Lazy, State } from '@openfn/lexicon'; import type { GetPlanReply, LightningPlan } from '@openfn/lexicon/lightning'; import type { Logger } from '@openfn/logger'; import { getWithReply } from '../util'; -import convertRun, { WorkerRunOptions } from '../util/convert-lightning-plan'; import { GET_PLAN } from '../events'; import type { Channel, Socket } from '../types'; @@ -20,9 +18,7 @@ const joinRunChannel = ( ) => { return new Promise<{ channel: Channel; - plan: ExecutionPlan; - options: WorkerRunOptions; - input: Lazy; + run: LightningPlan; }>((resolve, reject) => { // TMP - lightning seems to be sending two responses to me // just for now, I'm gonna gate the handling here @@ -38,9 +34,8 @@ const joinRunChannel = ( if (!didReceiveOk) { didReceiveOk = true; logger.success(`connected to ${channelName}`, e); - const { plan, options, input } = await loadRun(channel); - logger.debug('converted run as execution plan:', plan); - resolve({ channel, plan, options, input }); + const run = await getWithReply(channel, GET_PLAN); + resolve({ channel, run }); } }) .receive('error', (err: any) => { @@ -65,10 +60,3 @@ const joinRunChannel = ( }; export default joinRunChannel; - -export async function loadRun(channel: Channel) { - // first we get the run body through the socket - const runBody = await getWithReply(channel, GET_PLAN); - // then we generate the execution plan - return convertRun(runBody as LightningPlan); -} diff --git a/packages/ws-worker/src/server.ts b/packages/ws-worker/src/server.ts index d9562510f..0f729dd0b 100644 --- a/packages/ws-worker/src/server.ts +++ b/packages/ws-worker/src/server.ts @@ -5,7 +5,7 @@ import koaLogger from 'koa-logger'; import Router from '@koa/router'; import { humanId } from 'human-id'; import { createMockLogger, Logger } from '@openfn/logger'; -import { ClaimRun } from '@openfn/lexicon/lightning'; +import { ClaimRun, LightningPlan } from '@openfn/lexicon/lightning'; import { INTERNAL_RUN_COMPLETE } from './events'; import destroy from './api/destroy'; import startWorkloop, { Workloop } from './api/workloop'; @@ -18,6 +18,7 @@ import connectToWorkerQueue from './channels/worker-queue'; import type { Server } from 'http'; import type { RuntimeEngine } from '@openfn/engine-multi'; import type { Socket, Channel } from './types'; +import { convertRun } from './util'; export type ServerOptions = { maxWorkflows?: number; @@ -36,6 +37,7 @@ export type ServerOptions = { socketTimeoutSeconds?: number; payloadLimitMb?: number; // max memory limit for socket payload (ie, step:complete, log) + collectionsVersion?: string; }; // this is the server/koa API @@ -50,6 +52,9 @@ export interface ServerApp extends Koa { engine: RuntimeEngine; options: ServerOptions; workloop?: Workloop; + // What version of the collections adaptor should we use? + // Can be set through CLI, or else it'll look up latest on startup + collectionsVersion?: string; execute: ({ id, token }: ClaimRun) => Promise; destroy: () => void; @@ -137,6 +142,20 @@ function connect(app: ServerApp, logger: Logger, options: ServerOptions = {}) { .on('error', onError); } +async function lookupCollectionsVersion( + options: ServerOptions, + logger: Logger +) { + if (options.collectionsVersion && options.collectionsVersion !== 'latest') { + logger.log( + 'Using collections version from CLI/env: ', + options.collectionsVersion + ); + return options.collectionsVersion; + } + return; +} + function createServer(engine: RuntimeEngine, options: ServerOptions = {}) { const logger = options.logger || createMockLogger(); const port = options.port || DEFAULT_PORT; @@ -195,12 +214,18 @@ function createServer(engine: RuntimeEngine, options: ServerOptions = {}) { const start = Date.now(); app.workflows[id] = true; - const { - channel: runChannel, - plan, - options = {}, - input, - } = await joinRunChannel(app.socket, token, id, logger); + const { channel: runChannel, run } = await joinRunChannel( + app.socket, + token, + id, + logger + ); + + const { plan, options, input } = convertRun( + run, + app.options.collectionsVersion + ); + logger.debug('converted run body into execution plan:', plan); // Setup collections if (plan.workflow.credentials?.collections_token) { @@ -275,7 +300,10 @@ function createServer(engine: RuntimeEngine, options: ServerOptions = {}) { app.use(router.routes()); if (options.lightning) { - connect(app, logger, options); + lookupCollectionsVersion(options, logger).then((version) => { + app.collectionsVersion = version; + connect(app, logger, options); + }); } else { logger.warn('No lightning URL provided'); } diff --git a/packages/ws-worker/src/start.ts b/packages/ws-worker/src/start.ts index 56e22ad0a..301a1f5df 100644 --- a/packages/ws-worker/src/start.ts +++ b/packages/ws-worker/src/start.ts @@ -42,6 +42,7 @@ function engineReady(engine: any) { }, maxWorkflows: args.capacity, payloadLimitMb: args.payloadMemory, + collectionsVersion: args.collectionsVersion, }; if (args.lightningPublicKey) { diff --git a/packages/ws-worker/src/util/cli.ts b/packages/ws-worker/src/util/cli.ts index 1c7a2bf83..05d7d5682 100644 --- a/packages/ws-worker/src/util/cli.ts +++ b/packages/ws-worker/src/util/cli.ts @@ -23,6 +23,7 @@ type Args = { statePropsToRemove?: string[]; maxRunDurationSeconds: number; socketTimeoutSeconds?: number; + collectionsVersion?: string; }; type ArgTypes = string | string[] | number | undefined; @@ -51,6 +52,7 @@ export default function parseArgs(argv: string[]): Args { const { WORKER_BACKOFF, WORKER_CAPACITY, + WORKER_COLLECTIONS_VERSION, WORKER_LIGHTNING_PUBLIC_KEY, WORKER_LIGHTNING_SERVICE_URL, WORKER_LOG_LEVEL, @@ -135,6 +137,11 @@ export default function parseArgs(argv: string[]): Args { description: 'Default run timeout for the server, in seconds. Env: WORKER_MAX_RUN_DURATION_SECONDS', type: 'number', + }) + .option('collections-version', { + description: + 'The verison of the collections adaptor to use for all runs on this worker instance.Env: WORKER_COLLECTIONS_VERSION', + type: 'string', }); const args = parser.parse() as Args; @@ -173,5 +180,9 @@ export default function parseArgs(argv: string[]): Args { WORKER_SOCKET_TIMEOUT_SECONDS, DEFAULT_SOCKET_TIMEOUT_SECONDS ), + collectionsVersion: setArg( + args.collectionsVersion, + WORKER_COLLECTIONS_VERSION + ), } as Args; } diff --git a/packages/ws-worker/src/util/convert-lightning-plan.ts b/packages/ws-worker/src/util/convert-lightning-plan.ts index 01eeddcac..4e5402e97 100644 --- a/packages/ws-worker/src/util/convert-lightning-plan.ts +++ b/packages/ws-worker/src/util/convert-lightning-plan.ts @@ -41,14 +41,17 @@ const mapTriggerEdgeCondition = (edge: LightningEdge) => { // This function will look at every step and decide whether the collections adaptor // should be added to the array -const appendCollectionsAdaptor = (plan: ExecutionPlan) => { +const appendCollectionsAdaptor = ( + plan: ExecutionPlan, + collectionsVersion: string = 'latest' +) => { let hasCollections; plan.workflow.steps.forEach((step) => { const job = step as Job; if (job.expression?.match(/(collections\.)/)) { hasCollections = true; job.adaptors ??= []; - job.adaptors.push('@openfn/language-collections'); // what about version? Is this safe? + job.adaptors.push(`@openfn/language-collections@${collectionsVersion}`); } }); return hasCollections; @@ -62,7 +65,8 @@ export type WorkerRunOptions = ExecuteOptions & { }; export default ( - run: LightningPlan + run: LightningPlan, + collectionsVersion?: string ): { plan: ExecutionPlan; options: WorkerRunOptions; input: Lazy } => { // Some options get mapped straight through to the runtime's workflow options const runtimeOpts: Omit = {}; @@ -185,7 +189,10 @@ export default ( plan.workflow.name = run.name; } - const hasCollections = appendCollectionsAdaptor(plan as ExecutionPlan); + const hasCollections = appendCollectionsAdaptor( + plan as ExecutionPlan, + collectionsVersion + ); if (hasCollections) { plan.workflow.credentials = { collections_token: true, From e9e748b53fc22c3f63f42faf420242cc4d55b06a Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Wed, 23 Oct 2024 11:28:47 +0100 Subject: [PATCH 24/52] worker: lookup latest collections version on server start --- packages/ws-worker/src/server.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/ws-worker/src/server.ts b/packages/ws-worker/src/server.ts index 0f729dd0b..20231ff48 100644 --- a/packages/ws-worker/src/server.ts +++ b/packages/ws-worker/src/server.ts @@ -1,4 +1,8 @@ import { EventEmitter } from 'node:events'; + +import { promisify } from 'node:util'; +import { exec as _exec } from 'node:child_process'; + import Koa from 'koa'; import bodyParser from 'koa-bodyparser'; import koaLogger from 'koa-logger'; @@ -20,6 +24,8 @@ import type { RuntimeEngine } from '@openfn/engine-multi'; import type { Socket, Channel } from './types'; import { convertRun } from './util'; +const exec = promisify(_exec); + export type ServerOptions = { maxWorkflows?: number; port?: number; @@ -153,7 +159,11 @@ async function lookupCollectionsVersion( ); return options.collectionsVersion; } - return; + const { stdout: version } = await exec( + 'npm view @openfn/language-collections@next version' + ); + logger.log('Using collections version from @latest: ', version); + return version; } function createServer(engine: RuntimeEngine, options: ServerOptions = {}) { From bcd82e93d42e4e267999c3f86013d1231674c1ae Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Wed, 23 Oct 2024 11:29:31 +0100 Subject: [PATCH 25/52] worker: changeset --- .changeset/stale-taxis-attack.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/stale-taxis-attack.md diff --git a/.changeset/stale-taxis-attack.md b/.changeset/stale-taxis-attack.md new file mode 100644 index 000000000..5bc8c7341 --- /dev/null +++ b/.changeset/stale-taxis-attack.md @@ -0,0 +1,5 @@ +--- +'@openfn/ws-worker': minor +--- + +Support for collection versions (as arg or auto-looked-up on startup) From 294ca14cf54ad1349229af3055068286dba77bd5 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Wed, 23 Oct 2024 11:46:43 +0100 Subject: [PATCH 26/52] worker: update tests --- packages/ws-worker/test/channels/run.test.ts | 65 ++----------------- .../test/util/convert-lightning-plan.test.ts | 4 +- 2 files changed, 7 insertions(+), 62 deletions(-) diff --git a/packages/ws-worker/test/channels/run.test.ts b/packages/ws-worker/test/channels/run.test.ts index 4bbe192b6..71a76fe2a 100644 --- a/packages/ws-worker/test/channels/run.test.ts +++ b/packages/ws-worker/test/channels/run.test.ts @@ -1,79 +1,24 @@ import test from 'ava'; import { mockSocket, mockChannel } from '../../src/mock/sockets'; -import joinRunChannel, { loadRun } from '../../src/channels/run'; +import joinRunChannel from '../../src/channels/run'; import { GET_PLAN } from '../../src/events'; import { runs } from '../mock/data'; import { createMockLogger } from '@openfn/logger'; -test('loadRun should get the run body', async (t) => { - const run = runs['run-1']; - let didCallGetRun = false; - const channel = mockChannel({ - [GET_PLAN]: () => { - // TODO should be no payload (or empty payload) - didCallGetRun = true; - return run; - }, - }); - - await loadRun(channel); - t.true(didCallGetRun); -}); - -test('loadRun should return an execution plan and options', async (t) => { - const run = { - ...runs['run-1'], - options: { - sanitize: 'obfuscate', - run_timeout_ms: 10, - }, - }; - - const channel = mockChannel({ - [GET_PLAN]: () => run, - }); - - const { plan, options } = await loadRun(channel); - t.like(plan, { - id: 'run-1', - workflow: { - steps: [ - { - id: 'job-1', - configuration: 'a', - expression: 'fn(a => a)', - adaptors: ['@openfn/language-common@1.0.0'], - }, - ], - }, - }); - t.is(options.sanitize, 'obfuscate'); - t.is(options.runTimeoutMs, 10); -}); - -test('should join an run channel with a token', async (t) => { +test('should join a run channel with a token and return a raw lightning run', async (t) => { const logger = createMockLogger(); const socket = mockSocket('www', { 'run:a': mockChannel({ // Note that the validation logic is all handled here join: () => ({ status: 'ok' }), - [GET_PLAN]: () => ({ - id: 'a', - options: { run_timeout_ms: 10 }, - }), + [GET_PLAN]: () => runs['run-1'], }), }); - const { channel, plan, options } = await joinRunChannel( - socket, - 'x.y.z', - 'a', - logger - ); + const { channel, run } = await joinRunChannel(socket, 'x.y.z', 'a', logger); t.truthy(channel); - t.deepEqual(plan, { id: 'a', workflow: { steps: [] }, options: {} }); - t.deepEqual(options, { runTimeoutMs: 10 }); + t.deepEqual(run, runs['run-1']); }); test('should fail to join an run channel with an invalid token', async (t) => { diff --git a/packages/ws-worker/test/util/convert-lightning-plan.test.ts b/packages/ws-worker/test/util/convert-lightning-plan.test.ts index 578fe7ebd..78a6ef047 100644 --- a/packages/ws-worker/test/util/convert-lightning-plan.test.ts +++ b/packages/ws-worker/test/util/convert-lightning-plan.test.ts @@ -602,14 +602,14 @@ test('append the collections adaptor to jobs that use it', (t) => { triggers: [{ id: 't', type: 'cron' }], edges: [createEdge('a', 'b')], }; - const { plan } = convertPlan(run as LightningPlan); + const { plan } = convertPlan(run as LightningPlan, '1.0.0'); const [_t, a, b] = plan.workflow.steps; // @ts-ignore t.deepEqual(a.adaptors, ['common']); // @ts-ignore - t.deepEqual(b.adaptors, ['common', '@openfn/language-collections']); + t.deepEqual(b.adaptors, ['common', '@openfn/language-collections@1.0.0']); }); test('append the collections credential to jobs that use it', (t) => { From d4f084ef54c2d34e4b1dfa3325405a513a943f2c Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Wed, 23 Oct 2024 11:49:44 +0100 Subject: [PATCH 27/52] types --- packages/ws-worker/src/server.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ws-worker/src/server.ts b/packages/ws-worker/src/server.ts index 20231ff48..931ba3eb2 100644 --- a/packages/ws-worker/src/server.ts +++ b/packages/ws-worker/src/server.ts @@ -9,7 +9,7 @@ import koaLogger from 'koa-logger'; import Router from '@koa/router'; import { humanId } from 'human-id'; import { createMockLogger, Logger } from '@openfn/logger'; -import { ClaimRun, LightningPlan } from '@openfn/lexicon/lightning'; +import { ClaimRun } from '@openfn/lexicon/lightning'; import { INTERNAL_RUN_COMPLETE } from './events'; import destroy from './api/destroy'; import startWorkloop, { Workloop } from './api/workloop'; From 5799e17e1ca14527bca72449f97b5ad69e406b46 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Wed, 23 Oct 2024 12:39:45 +0100 Subject: [PATCH 28/52] tests: force collections token to stop lookups --- integration-tests/worker/src/init.ts | 1 + integration-tests/worker/test/server.test.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/integration-tests/worker/src/init.ts b/integration-tests/worker/src/init.ts index 9e1c768a3..b0cd63c3d 100644 --- a/integration-tests/worker/src/init.ts +++ b/integration-tests/worker/src/init.ts @@ -43,6 +43,7 @@ export const initWorker = async ( port: workerPort, lightning: `ws://localhost:${lightningPort}/worker`, secret: crypto.randomUUID(), + collectionsVersion: '1.0.0', ...workerArgs, }); diff --git a/integration-tests/worker/test/server.test.ts b/integration-tests/worker/test/server.test.ts index 4c22acba1..69ddcfeca 100644 --- a/integration-tests/worker/test/server.test.ts +++ b/integration-tests/worker/test/server.test.ts @@ -21,6 +21,7 @@ const spawnServer = (port: string | number = 1, args: string[] = []) => { '--backoff 0.001/0.01', '--log debug', '-s secretsquirrel', + '--collections-token=1.0.0', ...args, ], options From 6418aca5ad42589a9a7d96051388ab5427bc6804 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Wed, 23 Oct 2024 14:32:59 +0100 Subject: [PATCH 29/52] tests: add test for collections --- integration-tests/worker/src/init.ts | 1 + integration-tests/worker/test/runs.test.ts | 38 ++++++++++++++++++++-- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/integration-tests/worker/src/init.ts b/integration-tests/worker/src/init.ts index b0cd63c3d..358cc2687 100644 --- a/integration-tests/worker/src/init.ts +++ b/integration-tests/worker/src/init.ts @@ -5,6 +5,7 @@ import createLightningServer, { toBase64 } from '@openfn/lightning-mock'; import createEngine from '@openfn/engine-multi'; import createWorkerServer from '@openfn/ws-worker'; import { createMockLogger } from '@openfn/logger'; +// import createLogger from '@openfn/logger'; export const randomPort = () => Math.round(2000 + Math.random() * 1000); diff --git a/integration-tests/worker/test/runs.test.ts b/integration-tests/worker/test/runs.test.ts index 37caf94c8..7c15b29df 100644 --- a/integration-tests/worker/test/runs.test.ts +++ b/integration-tests/worker/test/runs.test.ts @@ -25,6 +25,7 @@ test.before(async () => { repoDir: path.resolve('tmp/repo/attempts'), }, { + collectionsVersion: '1.0.0-next-f802225c', runPublicKey: keys.public, } )); @@ -49,8 +50,8 @@ const run = async (t, attempt) => { // TODO friendlier job names for this would be nice (rather than run ids) t.log( `run ${payload.step_id} done in ${payload.duration / 1000}s [${humanMb( - payload.mem.job - )} / ${humanMb(payload.mem.system)}mb] [thread ${payload.thread_id}]` + payload.mem?.job + )} / ${humanMb(payload.mem?.system)}mb] [thread ${payload.thread_id}]` ); }); lightning.on('run:complete', (evt) => { @@ -248,3 +249,36 @@ test.serial('use different versions of the same adaptor', async (t) => { t.log(result); t.falsy(result.errors); }); + +test.serial('Run with collections', async (t) => { + const job1 = createJob({ + body: `fn((state = {}) => { + const server = collections.createMockServer(); + collections.setMockClient(server); + + server.api.createCollection('collection'); + + state.data = [{ id: 'a' }, { id: 'b' }, { id: 'c' }]; + state.results = []; + return state; + }); + + collections.set('collection', v => v.id, $.data); + + collections.each('collection', '*', (state, value, key) => { + state.results.push({ key, value }) + }); + `, + // Note: for some reason 1.7.0 fails because it exports a collections ?? + // 1.7.4 seems fine + adaptor: '@openfn/language-common@1.7.4', + }); + const attempt = createRun([], [job1], []); + + const { results } = await run(t, attempt); + t.deepEqual(results, [ + { key: 'a', value: { id: 'a' } }, + { key: 'b', value: { id: 'b' } }, + { key: 'c', value: { id: 'c' } }, + ]); +}); From 340a570cb64caa033c1a1df4f4a8b7125656b52c Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Wed, 23 Oct 2024 14:39:23 +0100 Subject: [PATCH 30/52] engine: remove .only --- packages/engine-multi/test/errors.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/engine-multi/test/errors.test.ts b/packages/engine-multi/test/errors.test.ts index 38d0979b1..0e4d7c7df 100644 --- a/packages/engine-multi/test/errors.test.ts +++ b/packages/engine-multi/test/errors.test.ts @@ -207,7 +207,7 @@ test.serial('after uncaught exception, free up the pool', (t) => { }); }); -test.serial.only('emit a crash error on process.exit()', (t) => { +test.serial('emit a crash error on process.exit()', (t) => { return new Promise((done) => { const plan = { id: 'z', From 02b92c4fc48ccd4bf45dcca51e22e9ee2f9f5ea9 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Wed, 23 Oct 2024 16:20:19 +0100 Subject: [PATCH 31/52] typo --- integration-tests/worker/test/server.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration-tests/worker/test/server.test.ts b/integration-tests/worker/test/server.test.ts index 69ddcfeca..9d403a406 100644 --- a/integration-tests/worker/test/server.test.ts +++ b/integration-tests/worker/test/server.test.ts @@ -21,7 +21,7 @@ const spawnServer = (port: string | number = 1, args: string[] = []) => { '--backoff 0.001/0.01', '--log debug', '-s secretsquirrel', - '--collections-token=1.0.0', + '--collections-version=1.0.0', ...args, ], options From c5c3e17a5d1d09db0c77c803e08bfea50297dfda Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Wed, 23 Oct 2024 16:26:38 +0100 Subject: [PATCH 32/52] worker: fix collections version --- packages/ws-worker/test/api/destroy.test.ts | 1 + packages/ws-worker/test/lightning.test.ts | 2 +- packages/ws-worker/test/server.test.ts | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/ws-worker/test/api/destroy.test.ts b/packages/ws-worker/test/api/destroy.test.ts index 2b47105c1..dce7c2992 100644 --- a/packages/ws-worker/test/api/destroy.test.ts +++ b/packages/ws-worker/test/api/destroy.test.ts @@ -24,6 +24,7 @@ test.beforeEach(async () => { lightning: `ws://localhost:${lightningPort}/worker`, port: workerPort, backoff: { min: 10, max: 20 }, + collectionsVersion: '1.0.0', }); }); diff --git a/packages/ws-worker/test/lightning.test.ts b/packages/ws-worker/test/lightning.test.ts index 4b508bcce..8c490e45f 100644 --- a/packages/ws-worker/test/lightning.test.ts +++ b/packages/ws-worker/test/lightning.test.ts @@ -42,7 +42,7 @@ test.before(async () => { lightning: urls.lng, secret: 'abc', maxWorkflows: 1, - + collectionsVersion: '1.0.0', // Note that if this is not passed, // JWT verification will be skipped runPublicKey: keys.public, diff --git a/packages/ws-worker/test/server.test.ts b/packages/ws-worker/test/server.test.ts index 1ac45a6cc..577dd04e2 100644 --- a/packages/ws-worker/test/server.test.ts +++ b/packages/ws-worker/test/server.test.ts @@ -8,6 +8,7 @@ test.before(async () => { port: 2323, secret: 'abc', maxWorkflows: 1, + collectionsVersion: '1.0.0', }); }); From d4e6b786f4b3fb189c272b90339943e202efd5f9 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Wed, 23 Oct 2024 16:59:53 +0100 Subject: [PATCH 33/52] collections: use latest rather than next --- packages/ws-worker/src/server.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ws-worker/src/server.ts b/packages/ws-worker/src/server.ts index 931ba3eb2..4a63cd277 100644 --- a/packages/ws-worker/src/server.ts +++ b/packages/ws-worker/src/server.ts @@ -160,7 +160,7 @@ async function lookupCollectionsVersion( return options.collectionsVersion; } const { stdout: version } = await exec( - 'npm view @openfn/language-collections@next version' + 'npm view @openfn/language-collections@latest version' ); logger.log('Using collections version from @latest: ', version); return version; From ae1d921da61de5db32111e3cab8211d3887c691c Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Wed, 23 Oct 2024 18:42:39 +0100 Subject: [PATCH 34/52] versions --- .changeset/eleven-cougars-exist.md | 5 ----- .changeset/healthy-laws-sniff.md | 5 ----- .changeset/loud-crabs-walk.md | 5 ----- .changeset/neat-lies-begin.md | 5 ----- .changeset/sharp-needles-peel.md | 5 ----- .changeset/sharp-tips-pretend.md | 5 ----- .changeset/spicy-parents-guess.md | 5 ----- .changeset/stale-taxis-attack.md | 5 ----- .changeset/wild-donuts-check.md | 5 ----- integration-tests/execute/CHANGELOG.md | 10 ++++++++++ integration-tests/execute/package.json | 2 +- integration-tests/worker/CHANGELOG.md | 13 +++++++++++++ integration-tests/worker/package.json | 2 +- packages/cli/CHANGELOG.md | 11 +++++++++++ packages/cli/package.json | 2 +- packages/compiler/CHANGELOG.md | 7 +++++++ packages/compiler/package.json | 2 +- packages/engine-multi/CHANGELOG.md | 14 ++++++++++++++ packages/engine-multi/package.json | 2 +- packages/lightning-mock/CHANGELOG.md | 9 +++++++++ packages/lightning-mock/package.json | 2 +- packages/runtime/CHANGELOG.md | 6 ++++++ packages/runtime/package.json | 2 +- packages/ws-worker/CHANGELOG.md | 16 ++++++++++++++++ packages/ws-worker/package.json | 2 +- 25 files changed, 94 insertions(+), 53 deletions(-) delete mode 100644 .changeset/eleven-cougars-exist.md delete mode 100644 .changeset/healthy-laws-sniff.md delete mode 100644 .changeset/loud-crabs-walk.md delete mode 100644 .changeset/neat-lies-begin.md delete mode 100644 .changeset/sharp-needles-peel.md delete mode 100644 .changeset/sharp-tips-pretend.md delete mode 100644 .changeset/spicy-parents-guess.md delete mode 100644 .changeset/stale-taxis-attack.md delete mode 100644 .changeset/wild-donuts-check.md diff --git a/.changeset/eleven-cougars-exist.md b/.changeset/eleven-cougars-exist.md deleted file mode 100644 index 53e2817d7..000000000 --- a/.changeset/eleven-cougars-exist.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@openfn/ws-worker': minor ---- - -Support collections diff --git a/.changeset/healthy-laws-sniff.md b/.changeset/healthy-laws-sniff.md deleted file mode 100644 index 70905b01e..000000000 --- a/.changeset/healthy-laws-sniff.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@openfn/runtime': major ---- - -Support global credential object on a workflow diff --git a/.changeset/loud-crabs-walk.md b/.changeset/loud-crabs-walk.md deleted file mode 100644 index 96c3ddfc5..000000000 --- a/.changeset/loud-crabs-walk.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@openfn/ws-worker': patch ---- - -Append the collections adaptor to steps that need it diff --git a/.changeset/neat-lies-begin.md b/.changeset/neat-lies-begin.md deleted file mode 100644 index 2fed34237..000000000 --- a/.changeset/neat-lies-begin.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@openfn/engine-multi': minor ---- - -Support multiple adaptors in job structures diff --git a/.changeset/sharp-needles-peel.md b/.changeset/sharp-needles-peel.md deleted file mode 100644 index 2a010c43f..000000000 --- a/.changeset/sharp-needles-peel.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@openfn/ws-worker': patch ---- - -Update worker to use adaptors as an array on xplans. Internal only change. diff --git a/.changeset/sharp-tips-pretend.md b/.changeset/sharp-tips-pretend.md deleted file mode 100644 index bf36342bd..000000000 --- a/.changeset/sharp-tips-pretend.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@openfn/compiler': minor ---- - -support multiple adaptors when adding imports diff --git a/.changeset/spicy-parents-guess.md b/.changeset/spicy-parents-guess.md deleted file mode 100644 index 14d13dbc0..000000000 --- a/.changeset/spicy-parents-guess.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@openfn/compiler': minor ---- - -Be more accurate in calculating exports from an adaptor" diff --git a/.changeset/stale-taxis-attack.md b/.changeset/stale-taxis-attack.md deleted file mode 100644 index 5bc8c7341..000000000 --- a/.changeset/stale-taxis-attack.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@openfn/ws-worker': minor ---- - -Support for collection versions (as arg or auto-looked-up on startup) diff --git a/.changeset/wild-donuts-check.md b/.changeset/wild-donuts-check.md deleted file mode 100644 index 741a40180..000000000 --- a/.changeset/wild-donuts-check.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@openfn/cli': patch ---- - -Support multiple adaptors diff --git a/integration-tests/execute/CHANGELOG.md b/integration-tests/execute/CHANGELOG.md index 4227ce99a..68e4e31cf 100644 --- a/integration-tests/execute/CHANGELOG.md +++ b/integration-tests/execute/CHANGELOG.md @@ -1,5 +1,15 @@ # @openfn/integration-tests-execute +## 1.0.6 + +### Patch Changes + +- Updated dependencies [3463ff9] +- Updated dependencies [7a85894] +- Updated dependencies [b6de2c4] + - @openfn/runtime@1.5.0 + - @openfn/compiler@0.4.0 + ## 1.0.5 ### Patch Changes diff --git a/integration-tests/execute/package.json b/integration-tests/execute/package.json index ad218cfcb..85e594a98 100644 --- a/integration-tests/execute/package.json +++ b/integration-tests/execute/package.json @@ -1,7 +1,7 @@ { "name": "@openfn/integration-tests-execute", "private": true, - "version": "1.0.5", + "version": "1.0.6", "description": "Job execution tests", "author": "Open Function Group ", "license": "ISC", diff --git a/integration-tests/worker/CHANGELOG.md b/integration-tests/worker/CHANGELOG.md index 67f321c2c..df7816a03 100644 --- a/integration-tests/worker/CHANGELOG.md +++ b/integration-tests/worker/CHANGELOG.md @@ -1,5 +1,18 @@ # @openfn/integration-tests-worker +## 1.0.63 + +### Patch Changes + +- Updated dependencies [fd0e499] +- Updated dependencies [1c79dc1] +- Updated dependencies [7245bf7] +- Updated dependencies [b15f151] +- Updated dependencies [bcd82e9] + - @openfn/ws-worker@1.8.0 + - @openfn/engine-multi@1.4.0 + - @openfn/lightning-mock@2.0.21 + ## 1.0.62 ### Patch Changes diff --git a/integration-tests/worker/package.json b/integration-tests/worker/package.json index 089ee40ae..8bb566c17 100644 --- a/integration-tests/worker/package.json +++ b/integration-tests/worker/package.json @@ -1,7 +1,7 @@ { "name": "@openfn/integration-tests-worker", "private": true, - "version": "1.0.62", + "version": "1.0.63", "description": "Lightning WOrker integration tests", "author": "Open Function Group ", "license": "ISC", diff --git a/packages/cli/CHANGELOG.md b/packages/cli/CHANGELOG.md index 2ddb6377b..f11db6ae4 100644 --- a/packages/cli/CHANGELOG.md +++ b/packages/cli/CHANGELOG.md @@ -1,5 +1,16 @@ # @openfn/cli +## 1.8.6 + +### Patch Changes + +- 528e9a0: Support multiple adaptors +- Updated dependencies [3463ff9] +- Updated dependencies [7a85894] +- Updated dependencies [b6de2c4] + - @openfn/runtime@1.5.0 + - @openfn/compiler@0.4.0 + ## 1.8.5 ### Patch Changes diff --git a/packages/cli/package.json b/packages/cli/package.json index 996d5250c..742a1b439 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -1,6 +1,6 @@ { "name": "@openfn/cli", - "version": "1.8.5", + "version": "1.8.6", "description": "CLI devtools for the openfn toolchain.", "engines": { "node": ">=18", diff --git a/packages/compiler/CHANGELOG.md b/packages/compiler/CHANGELOG.md index 442c9d38a..1be5521e3 100644 --- a/packages/compiler/CHANGELOG.md +++ b/packages/compiler/CHANGELOG.md @@ -1,5 +1,12 @@ # @openfn/compiler +## 0.4.0 + +### Minor Changes + +- 7a85894: support multiple adaptors when adding imports +- b6de2c4: Be more accurate in calculating exports from an adaptor + ## 0.3.3 ### Patch Changes diff --git a/packages/compiler/package.json b/packages/compiler/package.json index 2e0cd5131..a0fba4a7f 100644 --- a/packages/compiler/package.json +++ b/packages/compiler/package.json @@ -1,6 +1,6 @@ { "name": "@openfn/compiler", - "version": "0.3.3", + "version": "0.4.0", "description": "Compiler and language tooling for openfn jobs.", "author": "Open Function Group ", "license": "ISC", diff --git a/packages/engine-multi/CHANGELOG.md b/packages/engine-multi/CHANGELOG.md index 4622515d5..c52f05c7b 100644 --- a/packages/engine-multi/CHANGELOG.md +++ b/packages/engine-multi/CHANGELOG.md @@ -1,5 +1,19 @@ # engine-multi +## 1.4.0 + +### Minor Changes + +- 7245bf7: Support multiple adaptors in job structures + +### Patch Changes + +- Updated dependencies [3463ff9] +- Updated dependencies [7a85894] +- Updated dependencies [b6de2c4] + - @openfn/runtime@1.5.0 + - @openfn/compiler@0.4.0 + ## 1.3.0 ### Minor Changes diff --git a/packages/engine-multi/package.json b/packages/engine-multi/package.json index b61f23979..133a6af27 100644 --- a/packages/engine-multi/package.json +++ b/packages/engine-multi/package.json @@ -1,6 +1,6 @@ { "name": "@openfn/engine-multi", - "version": "1.3.0", + "version": "1.4.0", "description": "Multi-process runtime engine", "main": "dist/index.js", "type": "module", diff --git a/packages/lightning-mock/CHANGELOG.md b/packages/lightning-mock/CHANGELOG.md index 9786be28c..5d79c74d3 100644 --- a/packages/lightning-mock/CHANGELOG.md +++ b/packages/lightning-mock/CHANGELOG.md @@ -1,5 +1,14 @@ # @openfn/lightning-mock +## 2.0.21 + +### Patch Changes + +- Updated dependencies [3463ff9] +- Updated dependencies [7245bf7] + - @openfn/runtime@1.5.0 + - @openfn/engine-multi@1.4.0 + ## 2.0.20 ### Patch Changes diff --git a/packages/lightning-mock/package.json b/packages/lightning-mock/package.json index 18faf906a..2a9c14853 100644 --- a/packages/lightning-mock/package.json +++ b/packages/lightning-mock/package.json @@ -1,6 +1,6 @@ { "name": "@openfn/lightning-mock", - "version": "2.0.20", + "version": "2.0.21", "private": true, "description": "A mock Lightning server", "main": "dist/index.js", diff --git a/packages/runtime/CHANGELOG.md b/packages/runtime/CHANGELOG.md index 0e7f0f7d3..d9fd81dc5 100644 --- a/packages/runtime/CHANGELOG.md +++ b/packages/runtime/CHANGELOG.md @@ -1,5 +1,11 @@ # @openfn/runtime +## 1.5.0 + +### Minor Changes + +- 3463ff9: Support global credential object on a workflow + ## 1.4.2 ### Patch Changes diff --git a/packages/runtime/package.json b/packages/runtime/package.json index 8741d1fe5..f7172b47d 100644 --- a/packages/runtime/package.json +++ b/packages/runtime/package.json @@ -1,6 +1,6 @@ { "name": "@openfn/runtime", - "version": "1.4.2", + "version": "1.5.0", "description": "Job processing runtime.", "type": "module", "exports": { diff --git a/packages/ws-worker/CHANGELOG.md b/packages/ws-worker/CHANGELOG.md index 12f3005bd..a310eb866 100644 --- a/packages/ws-worker/CHANGELOG.md +++ b/packages/ws-worker/CHANGELOG.md @@ -1,5 +1,21 @@ # ws-worker +## 1.8.0 + +### Minor Changes + +- fd0e499: Support collections +- bcd82e9: Accept collection version at startup (as arg or auto-looked-up from npm) + +### Patch Changes + +- 1c79dc1: Append the collections adaptor to steps that need it +- b15f151: Update worker to use adaptors as an array on xplans. Internal only change. +- Updated dependencies [3463ff9] +- Updated dependencies [7245bf7] + - @openfn/runtime@1.5.0 + - @openfn/engine-multi@1.4.0 + ## 1.7.1 ### Patch Changes diff --git a/packages/ws-worker/package.json b/packages/ws-worker/package.json index cd0c2a8f3..eef4ddfef 100644 --- a/packages/ws-worker/package.json +++ b/packages/ws-worker/package.json @@ -1,6 +1,6 @@ { "name": "@openfn/ws-worker", - "version": "1.7.1", + "version": "1.8.0", "description": "A Websocket Worker to connect Lightning to a Runtime Engine", "main": "dist/index.js", "type": "module", From 353a64445f4b8646a333b3792efcc682484fcd13 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Mon, 28 Oct 2024 15:27:28 +0000 Subject: [PATCH 35/52] worker: support @local adaptor versions --- packages/ws-worker/src/server.ts | 8 +-- .../src/util/convert-lightning-plan.ts | 61 +++++++++++++------ .../test/util/convert-lightning-plan.test.ts | 31 +++++++++- 3 files changed, 75 insertions(+), 25 deletions(-) diff --git a/packages/ws-worker/src/server.ts b/packages/ws-worker/src/server.ts index 4a63cd277..d9a048665 100644 --- a/packages/ws-worker/src/server.ts +++ b/packages/ws-worker/src/server.ts @@ -231,10 +231,10 @@ function createServer(engine: RuntimeEngine, options: ServerOptions = {}) { logger ); - const { plan, options, input } = convertRun( - run, - app.options.collectionsVersion - ); + const { plan, options, input } = convertRun(run, { + collectionsVersion: app.options.collectionsVersion, + monorepoPath: process.env.OPENFN_REPO_DIR, // TODO wire this up properly + }); logger.debug('converted run body into execution plan:', plan); // Setup collections diff --git a/packages/ws-worker/src/util/convert-lightning-plan.ts b/packages/ws-worker/src/util/convert-lightning-plan.ts index 4e5402e97..7e7774b8c 100644 --- a/packages/ws-worker/src/util/convert-lightning-plan.ts +++ b/packages/ws-worker/src/util/convert-lightning-plan.ts @@ -1,4 +1,5 @@ import crypto from 'node:crypto'; +import path from 'node:path'; import type { Step, StepId, @@ -12,6 +13,7 @@ import type { } from '@openfn/lexicon'; import { LightningPlan, LightningEdge } from '@openfn/lexicon/lightning'; import { ExecuteOptions } from '@openfn/engine-multi'; +import { getNameAndVersion } from '@openfn/runtime'; export const conditions: Record string | null> = { @@ -39,24 +41,6 @@ const mapTriggerEdgeCondition = (edge: LightningEdge) => { return condition; }; -// This function will look at every step and decide whether the collections adaptor -// should be added to the array -const appendCollectionsAdaptor = ( - plan: ExecutionPlan, - collectionsVersion: string = 'latest' -) => { - let hasCollections; - plan.workflow.steps.forEach((step) => { - const job = step as Job; - if (job.expression?.match(/(collections\.)/)) { - hasCollections = true; - job.adaptors ??= []; - job.adaptors.push(`@openfn/language-collections@${collectionsVersion}`); - } - }); - return hasCollections; -}; - // Options which relate to this execution but are not part of the plan export type WorkerRunOptions = ExecuteOptions & { // Defaults to true - must be explicity false to stop dataclips being sent @@ -64,10 +48,47 @@ export type WorkerRunOptions = ExecuteOptions & { payloadLimitMb?: number; }; +type ConversionOptions = { + collectionsVersion?: string; + monorepoPath?: string; +}; + export default ( run: LightningPlan, - collectionsVersion?: string + options: ConversionOptions = {} ): { plan: ExecutionPlan; options: WorkerRunOptions; input: Lazy } => { + const { collectionsVersion, monorepoPath } = options; + + const getLocalVersion = (adaptor: string) => { + const { name, version } = getNameAndVersion(adaptor); + if (monorepoPath && version === 'local') { + const shortName = name.replace('@openfn/language-', ''); + const abspath = path.resolve(monorepoPath, 'packages', shortName); + return `${name}=${abspath}`; + } + return adaptor; + }; + + // This function will look at every step and decide whether the collections adaptor + // should be added to the array + const appendCollectionsAdaptor = ( + plan: ExecutionPlan, + collectionsVersion: string = 'latest' + ) => { + let hasCollections; + plan.workflow.steps.forEach((step) => { + const job = step as Job; + if (job.expression?.match(/(collections\.)/)) { + hasCollections = true; + job.adaptors ??= []; + job.adaptors.push( + getLocalVersion(`@openfn/language-collections@${collectionsVersion}`) + ); + } + }); + return hasCollections; + }; + // Some options get mapped straight through to the runtime's workflow options const runtimeOpts: Omit = {}; @@ -144,7 +165,7 @@ export default ( id, configuration: step.credential || step.credential_id, expression: step.body!, - adaptors: step.adaptor ? [step.adaptor] : [], + adaptors: step.adaptor ? [getLocalVersion(step.adaptor)] : [], }; if (step.name) { diff --git a/packages/ws-worker/test/util/convert-lightning-plan.test.ts b/packages/ws-worker/test/util/convert-lightning-plan.test.ts index 78a6ef047..28303cb79 100644 --- a/packages/ws-worker/test/util/convert-lightning-plan.test.ts +++ b/packages/ws-worker/test/util/convert-lightning-plan.test.ts @@ -602,7 +602,9 @@ test('append the collections adaptor to jobs that use it', (t) => { triggers: [{ id: 't', type: 'cron' }], edges: [createEdge('a', 'b')], }; - const { plan } = convertPlan(run as LightningPlan, '1.0.0'); + const { plan } = convertPlan(run as LightningPlan, { + collectionsVersion: '1.0.0', + }); const [_t, a, b] = plan.workflow.steps; @@ -634,3 +636,30 @@ test('append the collections credential to jobs that use it', (t) => { collections_endpoint: 'https://app.openfn.org', }); }); + +test('Use local paths', (t) => { + const run: Partial = { + id: 'w', + jobs: [ + createNode({ + id: 'a', + body: 'collections.each("c", "k", (state) => state)', + adaptor: 'common@local', + }), + ], + triggers: [{ id: 't', type: 'cron' }], + edges: [createEdge('t', 'a')], + }; + const { plan } = convertPlan(run as LightningPlan, { + collectionsVersion: 'local', + monorepoPath: '/adaptors', + }); + + const [_t, a] = plan.workflow.steps; + + // @ts-ignore + t.deepEqual(a.adaptors, [ + 'common=/adaptors/packages/common', + '@openfn/language-collections=/adaptors/packages/collections', + ]); +}); From a9f55fc5b26c0b8b4cc37407bcc80f3edaaa9712 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Mon, 28 Oct 2024 15:28:48 +0000 Subject: [PATCH 36/52] changeset --- .changeset/slow-parrots-divide.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/slow-parrots-divide.md diff --git a/.changeset/slow-parrots-divide.md b/.changeset/slow-parrots-divide.md new file mode 100644 index 000000000..5d3ea1371 --- /dev/null +++ b/.changeset/slow-parrots-divide.md @@ -0,0 +1,5 @@ +--- +'@openfn/ws-worker': minor +--- + +Support @local adaptor versions (which map to the monorepo) From fe5675a05e4010b754289a8d3018a81bccb45367 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Mon, 28 Oct 2024 15:35:32 +0000 Subject: [PATCH 37/52] worker: hook up monorepoDir argument --- packages/ws-worker/src/server.ts | 3 ++- packages/ws-worker/src/start.ts | 2 +- packages/ws-worker/src/util/cli.ts | 30 +++++++++++++++++++----------- 3 files changed, 22 insertions(+), 13 deletions(-) diff --git a/packages/ws-worker/src/server.ts b/packages/ws-worker/src/server.ts index d9a048665..a7912e198 100644 --- a/packages/ws-worker/src/server.ts +++ b/packages/ws-worker/src/server.ts @@ -44,6 +44,7 @@ export type ServerOptions = { socketTimeoutSeconds?: number; payloadLimitMb?: number; // max memory limit for socket payload (ie, step:complete, log) collectionsVersion?: string; + monorepoDir?: string; }; // this is the server/koa API @@ -233,7 +234,7 @@ function createServer(engine: RuntimeEngine, options: ServerOptions = {}) { const { plan, options, input } = convertRun(run, { collectionsVersion: app.options.collectionsVersion, - monorepoPath: process.env.OPENFN_REPO_DIR, // TODO wire this up properly + monorepoPath: app.options.monorepoDir, }); logger.debug('converted run body into execution plan:', plan); diff --git a/packages/ws-worker/src/start.ts b/packages/ws-worker/src/start.ts index 301a1f5df..fb6bb924c 100644 --- a/packages/ws-worker/src/start.ts +++ b/packages/ws-worker/src/start.ts @@ -28,7 +28,6 @@ const [minBackoff, maxBackoff] = args.backoff function engineReady(engine: any) { logger.debug('Creating worker instance'); - const workerOptions: ServerOptions = { port: args.port, lightning: args.lightning, @@ -43,6 +42,7 @@ function engineReady(engine: any) { maxWorkflows: args.capacity, payloadLimitMb: args.payloadMemory, collectionsVersion: args.collectionsVersion, + monorepoDir: args.monorepoDir, }; if (args.lightningPublicKey) { diff --git a/packages/ws-worker/src/util/cli.ts b/packages/ws-worker/src/util/cli.ts index 05d7d5682..8b656b08d 100644 --- a/packages/ws-worker/src/util/cli.ts +++ b/packages/ws-worker/src/util/cli.ts @@ -8,22 +8,23 @@ const DEFAULT_SOCKET_TIMEOUT_SECONDS = 10; type Args = { _: string[]; - port?: number; + backoff: string; + capacity?: number; + collectionsVersion?: string; lightning?: string; - repoDir?: string; - secret?: string; - loop?: boolean; - log?: LogLevel; lightningPublicKey?: string; + log?: LogLevel; + loop?: boolean; + maxRunDurationSeconds: number; mock?: boolean; - backoff: string; - capacity?: number; - runMemory?: number; + monorepoDir?: string; payloadMemory?: number; - statePropsToRemove?: string[]; - maxRunDurationSeconds: number; + port?: number; + repoDir?: string; + runMemory?: number; + secret?: string; socketTimeoutSeconds?: number; - collectionsVersion?: string; + statePropsToRemove?: string[]; }; type ArgTypes = string | string[] | number | undefined; @@ -64,6 +65,7 @@ export default function parseArgs(argv: string[]): Args { WORKER_SECRET, WORKER_STATE_PROPS_TO_REMOVE, WORKER_SOCKET_TIMEOUT_SECONDS, + OPENFN_REPO_DIR, } = process.env; const parser = yargs(hideBin(argv)) @@ -83,6 +85,11 @@ export default function parseArgs(argv: string[]): Args { description: 'Path to the runtime repo (where modules will be installed). Env: WORKER_REPO_DIR', }) + .option('monorepo-dir', { + alias: 'm', + description: + 'Path to the adaptors mono repo, from where @local adaptors will be loaded. Env: OPENFN_REPO_DIR', + }) .option('secret', { alias: 's', description: @@ -155,6 +162,7 @@ export default function parseArgs(argv: string[]): Args { 'ws://localhost:4000/worker' ), repoDir: setArg(args.repoDir, WORKER_REPO_DIR), + monorepoDir: setArg(args.monorepoDir, OPENFN_REPO_DIR), secret: setArg(args.secret, WORKER_SECRET), lightningPublicKey: setArg( args.lightningPublicKey, From ac459845b77d1598bfd4cd14c73023fb24d58d0c Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Mon, 28 Oct 2024 16:44:15 +0000 Subject: [PATCH 38/52] runtime: allow a specifier to include a file path --- packages/runtime/src/modules/linker.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/runtime/src/modules/linker.ts b/packages/runtime/src/modules/linker.ts index 18a9cce2f..88d81a2fd 100644 --- a/packages/runtime/src/modules/linker.ts +++ b/packages/runtime/src/modules/linker.ts @@ -120,7 +120,11 @@ const loadActualModule = async (specifier: string, options: LinkerOptions) => { let path; let version; - if (options.modules?.[specifier]) { + // If the specifier contains a path, import it + // (this can happen if a job has a specific path ovveride) + if (specifier.includes('=')) { + path = specifier.split('=')[1]; + } else if (options.modules?.[specifier]) { ({ path, version } = options.modules?.[specifier]); } From 543556dffe74cb333dc5dbc305f288d50606b8ec Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Mon, 28 Oct 2024 16:46:40 +0000 Subject: [PATCH 39/52] runtime:test --- packages/runtime/test/modules/linker.test.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/runtime/test/modules/linker.test.ts b/packages/runtime/test/modules/linker.test.ts index 813a38e56..a2fef1c08 100644 --- a/packages/runtime/test/modules/linker.test.ts +++ b/packages/runtime/test/modules/linker.test.ts @@ -105,6 +105,16 @@ test('loads a module from a path', async (t) => { t.assert(m.namespace.default === 'test'); }); +// This is how the worker wants to do it: +// A module with a path will be passed on one step, +// rather than mapped globally +// TODO: double check this +test('loads a module from a path in the specifier', async (t) => { + const specifier = `ultimate-answer=${path.resolve('test/__modules__/test')}`; + const m = await linker(specifier, context, {}); + t.assert(m.namespace.default === 'test'); +}); + test('imports with a cacheKey', async (t) => { const opts = { ...options, From 7d0f8b97981151b85c419701b64b668986cd1c85 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Mon, 28 Oct 2024 16:47:46 +0000 Subject: [PATCH 40/52] runtime: changeset --- .changeset/tricky-olives-pay.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/tricky-olives-pay.md diff --git a/.changeset/tricky-olives-pay.md b/.changeset/tricky-olives-pay.md new file mode 100644 index 000000000..bbbbe58fc --- /dev/null +++ b/.changeset/tricky-olives-pay.md @@ -0,0 +1,5 @@ +--- +'@openfn/runtime': minor +--- + +Allow the linker to load a specifier with a path, ie, common=/a/b/c. This is useful for the worker to load @local modules on a per-step basis. From 2232261bc6005c84d9153b375f1a2c7030a1dbcd Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Mon, 28 Oct 2024 16:49:40 +0000 Subject: [PATCH 41/52] engine: don't try to autoinstall adaptors with an explicit path --- .changeset/wise-drinks-behave.md | 5 +++++ packages/engine-multi/src/api/autoinstall.ts | 4 ++++ 2 files changed, 9 insertions(+) create mode 100644 .changeset/wise-drinks-behave.md diff --git a/.changeset/wise-drinks-behave.md b/.changeset/wise-drinks-behave.md new file mode 100644 index 000000000..b213aa97b --- /dev/null +++ b/.changeset/wise-drinks-behave.md @@ -0,0 +1,5 @@ +--- +'@openfn/engine-multi': patch +--- + +Engine: don't try to autoinstall adaptor versions with an explicit path diff --git a/packages/engine-multi/src/api/autoinstall.ts b/packages/engine-multi/src/api/autoinstall.ts index 8e165d531..04798a6ec 100644 --- a/packages/engine-multi/src/api/autoinstall.ts +++ b/packages/engine-multi/src/api/autoinstall.ts @@ -61,6 +61,10 @@ const autoinstall = async (context: ExecutionContext): Promise => { ) => { // Check whether we still need to do any work for (const a of adaptors) { + if (a.match('=')) { + // Ignore adaptors with explicit paths (ie monorepo @local) + continue; + } const { name, version } = getNameAndVersion(a); if (await isInstalledFn(a, repoDir, logger)) { continue; From d3b39b60b429c6e391ab604be4e775bd60178a83 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Mon, 28 Oct 2024 16:49:57 +0000 Subject: [PATCH 42/52] worker: fix env var --- packages/ws-worker/src/util/cli.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/ws-worker/src/util/cli.ts b/packages/ws-worker/src/util/cli.ts index 8b656b08d..38d23b533 100644 --- a/packages/ws-worker/src/util/cli.ts +++ b/packages/ws-worker/src/util/cli.ts @@ -65,7 +65,7 @@ export default function parseArgs(argv: string[]): Args { WORKER_SECRET, WORKER_STATE_PROPS_TO_REMOVE, WORKER_SOCKET_TIMEOUT_SECONDS, - OPENFN_REPO_DIR, + OPENFN_ADAPTORS_REPO, } = process.env; const parser = yargs(hideBin(argv)) @@ -88,7 +88,7 @@ export default function parseArgs(argv: string[]): Args { .option('monorepo-dir', { alias: 'm', description: - 'Path to the adaptors mono repo, from where @local adaptors will be loaded. Env: OPENFN_REPO_DIR', + 'Path to the adaptors mono repo, from where @local adaptors will be loaded. Env: OPENFN_ADAPTORS_REPO', }) .option('secret', { alias: 's', @@ -162,7 +162,7 @@ export default function parseArgs(argv: string[]): Args { 'ws://localhost:4000/worker' ), repoDir: setArg(args.repoDir, WORKER_REPO_DIR), - monorepoDir: setArg(args.monorepoDir, OPENFN_REPO_DIR), + monorepoDir: setArg(args.monorepoDir, OPENFN_ADAPTORS_REPO), secret: setArg(args.secret, WORKER_SECRET), lightningPublicKey: setArg( args.lightningPublicKey, From 76f8c7cb74730b81e9fe60ceb730fc553307a97d Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Tue, 29 Oct 2024 12:32:07 +0000 Subject: [PATCH 43/52] worker: drive collections url from a new option --- packages/ws-worker/src/server.ts | 19 +++++++++++------ packages/ws-worker/src/start.ts | 1 + packages/ws-worker/src/util/cli.ts | 9 +++++++- .../src/util/convert-lightning-plan.ts | 20 ++++++++++-------- .../test/util/convert-lightning-plan.test.ts | 21 ++++++++++++++++++- 5 files changed, 53 insertions(+), 17 deletions(-) diff --git a/packages/ws-worker/src/server.ts b/packages/ws-worker/src/server.ts index 4a63cd277..f543dc288 100644 --- a/packages/ws-worker/src/server.ts +++ b/packages/ws-worker/src/server.ts @@ -44,6 +44,7 @@ export type ServerOptions = { socketTimeoutSeconds?: number; payloadLimitMb?: number; // max memory limit for socket payload (ie, step:complete, log) collectionsVersion?: string; + collectionsUrl?: string; }; // this is the server/koa API @@ -148,10 +149,16 @@ function connect(app: ServerApp, logger: Logger, options: ServerOptions = {}) { .on('error', onError); } -async function lookupCollectionsVersion( - options: ServerOptions, - logger: Logger -) { +async function setupCollections(options: ServerOptions, logger: Logger) { + if (!options.collectionsUrl) { + logger.warn( + 'WARNING: no collections URL provided. Collections service will not be enabled.' + ); + logger.warn( + 'Pass --collections-version or set WORKER_COLLECTIONS_URL to set the url' + ); + return; + } if (options.collectionsVersion && options.collectionsVersion !== 'latest') { logger.log( 'Using collections version from CLI/env: ', @@ -243,7 +250,7 @@ function createServer(engine: RuntimeEngine, options: ServerOptions = {}) { } if (plan.workflow.credentials?.collections_endpoint) { plan.workflow.credentials.collections_endpoint = - app.options.lightning; + app.options.collectionsUrl; } // Default the payload limit if it's not otherwise set on the run options @@ -310,7 +317,7 @@ function createServer(engine: RuntimeEngine, options: ServerOptions = {}) { app.use(router.routes()); if (options.lightning) { - lookupCollectionsVersion(options, logger).then((version) => { + setupCollections(options, logger).then((version) => { app.collectionsVersion = version; connect(app, logger, options); }); diff --git a/packages/ws-worker/src/start.ts b/packages/ws-worker/src/start.ts index 301a1f5df..78d1f372f 100644 --- a/packages/ws-worker/src/start.ts +++ b/packages/ws-worker/src/start.ts @@ -43,6 +43,7 @@ function engineReady(engine: any) { maxWorkflows: args.capacity, payloadLimitMb: args.payloadMemory, collectionsVersion: args.collectionsVersion, + collectionsUrl: args.collectionsUrl, }; if (args.lightningPublicKey) { diff --git a/packages/ws-worker/src/util/cli.ts b/packages/ws-worker/src/util/cli.ts index 05d7d5682..3f2e0df39 100644 --- a/packages/ws-worker/src/util/cli.ts +++ b/packages/ws-worker/src/util/cli.ts @@ -24,6 +24,7 @@ type Args = { maxRunDurationSeconds: number; socketTimeoutSeconds?: number; collectionsVersion?: string; + collectionsUrl?: string; }; type ArgTypes = string | string[] | number | undefined; @@ -138,9 +139,14 @@ export default function parseArgs(argv: string[]): Args { 'Default run timeout for the server, in seconds. Env: WORKER_MAX_RUN_DURATION_SECONDS', type: 'number', }) + .option('collections-url', { + alias: ['c'], + description: + 'URL to the Collections service endpoint. Required for Collections.Env: WORKER_COLLECTIONS_URL', + }) .option('collections-version', { description: - 'The verison of the collections adaptor to use for all runs on this worker instance.Env: WORKER_COLLECTIONS_VERSION', + 'The version of the collections adaptor to use for all runs on this worker instance.Env: WORKER_COLLECTIONS_VERSION', type: 'string', }); @@ -184,5 +190,6 @@ export default function parseArgs(argv: string[]): Args { args.collectionsVersion, WORKER_COLLECTIONS_VERSION ), + collectionsUrl: setArg(args.collectionsUrl, WORKER_COLLECTIONS_URL), } as Args; } diff --git a/packages/ws-worker/src/util/convert-lightning-plan.ts b/packages/ws-worker/src/util/convert-lightning-plan.ts index 4e5402e97..f4d16a753 100644 --- a/packages/ws-worker/src/util/convert-lightning-plan.ts +++ b/packages/ws-worker/src/util/convert-lightning-plan.ts @@ -189,15 +189,17 @@ export default ( plan.workflow.name = run.name; } - const hasCollections = appendCollectionsAdaptor( - plan as ExecutionPlan, - collectionsVersion - ); - if (hasCollections) { - plan.workflow.credentials = { - collections_token: true, - collections_endpoint: 'https://app.openfn.org', - }; + if (collectionsVersion) { + const hasCollections = appendCollectionsAdaptor( + plan as ExecutionPlan, + collectionsVersion + ); + if (hasCollections) { + plan.workflow.credentials = { + collections_token: true, + collections_endpoint: 'https://app.openfn.org', + }; + } } return { diff --git a/packages/ws-worker/test/util/convert-lightning-plan.test.ts b/packages/ws-worker/test/util/convert-lightning-plan.test.ts index 78a6ef047..3df7eeebe 100644 --- a/packages/ws-worker/test/util/convert-lightning-plan.test.ts +++ b/packages/ws-worker/test/util/convert-lightning-plan.test.ts @@ -625,7 +625,7 @@ test('append the collections credential to jobs that use it', (t) => { triggers: [{ id: 't', type: 'cron' }], edges: [createEdge('a', 'b')], }; - const { plan } = convertPlan(run as LightningPlan); + const { plan } = convertPlan(run as LightningPlan, '1.0.0'); const creds = plan.workflow.credentials; @@ -634,3 +634,22 @@ test('append the collections credential to jobs that use it', (t) => { collections_endpoint: 'https://app.openfn.org', }); }); + +test("Don't set up collections if no version is passed", (t) => { + const run: Partial = { + id: 'w', + jobs: [ + createNode({ + id: 'a', + body: 'collections.each("c", "k", (state) => state)', + }), + ], + triggers: [{ id: 't', type: 'cron' }], + edges: [createEdge('t', 'a')], + }; + const { plan } = convertPlan(run as LightningPlan); + + const [_t, a] = plan.workflow.steps; + + t.deepEqual((a as Job).adaptors, ['common']); +}); From 3332576d229e42c87d2a86732b797566f5dbe6fd Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Tue, 29 Oct 2024 12:41:13 +0000 Subject: [PATCH 44/52] worker: logging around collections url --- packages/ws-worker/src/server.ts | 5 ++++- packages/ws-worker/src/util/cli.ts | 3 ++- packages/ws-worker/src/util/convert-lightning-plan.ts | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/ws-worker/src/server.ts b/packages/ws-worker/src/server.ts index f543dc288..376e27eb8 100644 --- a/packages/ws-worker/src/server.ts +++ b/packages/ws-worker/src/server.ts @@ -150,7 +150,9 @@ function connect(app: ServerApp, logger: Logger, options: ServerOptions = {}) { } async function setupCollections(options: ServerOptions, logger: Logger) { - if (!options.collectionsUrl) { + if (options.collectionsUrl) { + logger.log('Using collections endpoint at ', options.collectionsUrl); + } else { logger.warn( 'WARNING: no collections URL provided. Collections service will not be enabled.' ); @@ -159,6 +161,7 @@ async function setupCollections(options: ServerOptions, logger: Logger) { ); return; } + if (options.collectionsVersion && options.collectionsVersion !== 'latest') { logger.log( 'Using collections version from CLI/env: ', diff --git a/packages/ws-worker/src/util/cli.ts b/packages/ws-worker/src/util/cli.ts index 3f2e0df39..061a131f4 100644 --- a/packages/ws-worker/src/util/cli.ts +++ b/packages/ws-worker/src/util/cli.ts @@ -54,6 +54,7 @@ export default function parseArgs(argv: string[]): Args { WORKER_BACKOFF, WORKER_CAPACITY, WORKER_COLLECTIONS_VERSION, + WORKER_COLLECTIONS_URL, WORKER_LIGHTNING_PUBLIC_KEY, WORKER_LIGHTNING_SERVICE_URL, WORKER_LOG_LEVEL, @@ -142,7 +143,7 @@ export default function parseArgs(argv: string[]): Args { .option('collections-url', { alias: ['c'], description: - 'URL to the Collections service endpoint. Required for Collections.Env: WORKER_COLLECTIONS_URL', + 'URL to the Collections service endpoint. Required for Collections, eg, https://app.openfn.org/collections. Env: WORKER_COLLECTIONS_URL', }) .option('collections-version', { description: diff --git a/packages/ws-worker/src/util/convert-lightning-plan.ts b/packages/ws-worker/src/util/convert-lightning-plan.ts index f4d16a753..4624682f1 100644 --- a/packages/ws-worker/src/util/convert-lightning-plan.ts +++ b/packages/ws-worker/src/util/convert-lightning-plan.ts @@ -197,7 +197,7 @@ export default ( if (hasCollections) { plan.workflow.credentials = { collections_token: true, - collections_endpoint: 'https://app.openfn.org', + collections_endpoint: true, }; } } From 563470b27eb52acfacb8e6d4a3042565928f3902 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Tue, 29 Oct 2024 17:30:07 +0000 Subject: [PATCH 45/52] cleaner implementation of local adaptor paths --- packages/engine-multi/src/api/autoinstall.ts | 5 +++ .../src/util/convert-lightning-plan.ts | 33 ++++++++++++------- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/packages/engine-multi/src/api/autoinstall.ts b/packages/engine-multi/src/api/autoinstall.ts index 04798a6ec..22d0aa6f2 100644 --- a/packages/engine-multi/src/api/autoinstall.ts +++ b/packages/engine-multi/src/api/autoinstall.ts @@ -158,6 +158,11 @@ const autoinstall = async (context: ExecutionContext): Promise => { (context.versions[name] as string[]).push(v); } + if (v === 'local') { + logger.info('Using local version of ', a); + continue; + } + // important: write back to paths with the RAW specifier paths[a] = { path: `${repoDir}/node_modules/${alias}`, diff --git a/packages/ws-worker/src/util/convert-lightning-plan.ts b/packages/ws-worker/src/util/convert-lightning-plan.ts index d60ff6f10..45a5e52e7 100644 --- a/packages/ws-worker/src/util/convert-lightning-plan.ts +++ b/packages/ws-worker/src/util/convert-lightning-plan.ts @@ -59,14 +59,22 @@ export default ( ): { plan: ExecutionPlan; options: WorkerRunOptions; input: Lazy } => { const { collectionsVersion, monorepoPath } = options; - const getLocalVersion = (adaptor: string) => { - const { name, version } = getNameAndVersion(adaptor); - if (monorepoPath && version === 'local') { - const shortName = name.replace('@openfn/language-', ''); - const abspath = path.resolve(monorepoPath, 'packages', shortName); - return `${name}=${abspath}`; + const appendLocalVersions = (job: Job) => { + if (monorepoPath && job.adaptors!) { + for (const adaptor of job.adaptors) { + const { name, version } = getNameAndVersion(adaptor); + if (monorepoPath && version === 'local') { + const shortName = name.replace('@openfn/language-', ''); + const localPath = path.resolve(monorepoPath, 'packages', shortName); + job.linker ??= {}; + job.linker[name] = { + path: localPath, + version: 'local', + }; + } + } } - return adaptor; + return job; }; // This function will look at every step and decide whether the collections adaptor @@ -81,9 +89,7 @@ export default ( if (job.expression?.match(/(collections\.)/)) { hasCollections = true; job.adaptors ??= []; - job.adaptors.push( - getLocalVersion(`@openfn/language-collections@${collectionsVersion}`) - ); + job.adaptors.push(`@openfn/language-collections@${collectionsVersion}`); } }); return hasCollections; @@ -165,7 +171,7 @@ export default ( id, configuration: step.credential || step.credential_id, expression: step.body!, - adaptors: step.adaptor ? [getLocalVersion(step.adaptor)] : [], + adaptors: step.adaptor ? [step.adaptor] : [], }; if (step.name) { @@ -223,6 +229,11 @@ export default ( } } + // Find any @local versions and set them up properly + for (const step of plan.workflow.steps) { + appendLocalVersions(step as Job); + } + return { plan: plan as ExecutionPlan, options: engineOpts, From 42df98adb1f88927ebf3c32ad281aabec62141cf Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Tue, 29 Oct 2024 17:30:45 +0000 Subject: [PATCH 46/52] runtime: revert linker change --- .changeset/tricky-olives-pay.md | 5 ----- packages/runtime/src/modules/linker.ts | 6 +----- 2 files changed, 1 insertion(+), 10 deletions(-) delete mode 100644 .changeset/tricky-olives-pay.md diff --git a/.changeset/tricky-olives-pay.md b/.changeset/tricky-olives-pay.md deleted file mode 100644 index bbbbe58fc..000000000 --- a/.changeset/tricky-olives-pay.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@openfn/runtime': minor ---- - -Allow the linker to load a specifier with a path, ie, common=/a/b/c. This is useful for the worker to load @local modules on a per-step basis. diff --git a/packages/runtime/src/modules/linker.ts b/packages/runtime/src/modules/linker.ts index 88d81a2fd..18a9cce2f 100644 --- a/packages/runtime/src/modules/linker.ts +++ b/packages/runtime/src/modules/linker.ts @@ -120,11 +120,7 @@ const loadActualModule = async (specifier: string, options: LinkerOptions) => { let path; let version; - // If the specifier contains a path, import it - // (this can happen if a job has a specific path ovveride) - if (specifier.includes('=')) { - path = specifier.split('=')[1]; - } else if (options.modules?.[specifier]) { + if (options.modules?.[specifier]) { ({ path, version } = options.modules?.[specifier]); } From 914a84c8e0387ea0ebcaa50106817cc99d909eaa Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Tue, 29 Oct 2024 17:32:05 +0000 Subject: [PATCH 47/52] more cleanup --- .changeset/wise-drinks-behave.md | 2 +- packages/engine-multi/src/api/autoinstall.ts | 4 ---- packages/runtime/test/modules/linker.test.ts | 10 ---------- 3 files changed, 1 insertion(+), 15 deletions(-) diff --git a/.changeset/wise-drinks-behave.md b/.changeset/wise-drinks-behave.md index b213aa97b..bdc402950 100644 --- a/.changeset/wise-drinks-behave.md +++ b/.changeset/wise-drinks-behave.md @@ -2,4 +2,4 @@ '@openfn/engine-multi': patch --- -Engine: don't try to autoinstall adaptor versions with an explicit path +Engine: don't try to autoinstall adaptor versions with @local diff --git a/packages/engine-multi/src/api/autoinstall.ts b/packages/engine-multi/src/api/autoinstall.ts index 22d0aa6f2..bf4bae9a0 100644 --- a/packages/engine-multi/src/api/autoinstall.ts +++ b/packages/engine-multi/src/api/autoinstall.ts @@ -61,10 +61,6 @@ const autoinstall = async (context: ExecutionContext): Promise => { ) => { // Check whether we still need to do any work for (const a of adaptors) { - if (a.match('=')) { - // Ignore adaptors with explicit paths (ie monorepo @local) - continue; - } const { name, version } = getNameAndVersion(a); if (await isInstalledFn(a, repoDir, logger)) { continue; diff --git a/packages/runtime/test/modules/linker.test.ts b/packages/runtime/test/modules/linker.test.ts index a2fef1c08..813a38e56 100644 --- a/packages/runtime/test/modules/linker.test.ts +++ b/packages/runtime/test/modules/linker.test.ts @@ -105,16 +105,6 @@ test('loads a module from a path', async (t) => { t.assert(m.namespace.default === 'test'); }); -// This is how the worker wants to do it: -// A module with a path will be passed on one step, -// rather than mapped globally -// TODO: double check this -test('loads a module from a path in the specifier', async (t) => { - const specifier = `ultimate-answer=${path.resolve('test/__modules__/test')}`; - const m = await linker(specifier, context, {}); - t.assert(m.namespace.default === 'test'); -}); - test('imports with a cacheKey', async (t) => { const opts = { ...options, From 6e6d1242def7589bed4e705e0bca235d7ef7ea17 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Tue, 29 Oct 2024 17:46:03 +0000 Subject: [PATCH 48/52] update tests --- .../test/util/convert-lightning-plan.test.ts | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/packages/ws-worker/test/util/convert-lightning-plan.test.ts b/packages/ws-worker/test/util/convert-lightning-plan.test.ts index e4540d276..ed8f9da31 100644 --- a/packages/ws-worker/test/util/convert-lightning-plan.test.ts +++ b/packages/ws-worker/test/util/convert-lightning-plan.test.ts @@ -679,11 +679,21 @@ test('Use local paths', (t) => { monorepoPath: '/adaptors', }); - const [_t, a] = plan.workflow.steps; + const [_t, a] = plan.workflow.steps as any[]; - // @ts-ignore t.deepEqual(a.adaptors, [ - 'common=/adaptors/packages/common', - '@openfn/language-collections=/adaptors/packages/collections', + 'common@local', + '@openfn/language-collections@local', ]); + t.deepEqual(a.linker, { + // The adaptor is not exapanded into long form, could be a problem + common: { + path: '/adaptors/packages/common', + version: 'local', + }, + '@openfn/language-collections': { + path: '/adaptors/packages/collections', + version: 'local', + }, + }); }); From 669f9cbbe7403c8cdf4bf96b6ba97d0fe6205ca2 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Tue, 29 Oct 2024 18:23:59 +0000 Subject: [PATCH 49/52] tests: integration test for worker monorepo --- .../worker/monorepo/packages/common/index.js | 8 ++++++ .../monorepo/packages/common/package.json | 7 +++++ .../worker/test/integration.test.ts | 26 ++++++++++++++++++- 3 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 integration-tests/worker/monorepo/packages/common/index.js create mode 100644 integration-tests/worker/monorepo/packages/common/package.json diff --git a/integration-tests/worker/monorepo/packages/common/index.js b/integration-tests/worker/monorepo/packages/common/index.js new file mode 100644 index 000000000..0406060fa --- /dev/null +++ b/integration-tests/worker/monorepo/packages/common/index.js @@ -0,0 +1,8 @@ +function fortyTwo() { + return (state) => { + state.data = 42; + return state; + }; +} + +module.exports = { fortyTwo }; diff --git a/integration-tests/worker/monorepo/packages/common/package.json b/integration-tests/worker/monorepo/packages/common/package.json new file mode 100644 index 000000000..e4b68cfec --- /dev/null +++ b/integration-tests/worker/monorepo/packages/common/package.json @@ -0,0 +1,7 @@ +{ + "name": "@openfn/language-common", + "private": true, + "version": "1.0.0", + "dependencies": {}, + "devDependencies": {} +} diff --git a/integration-tests/worker/test/integration.test.ts b/integration-tests/worker/test/integration.test.ts index 57274819a..2d29e8ea9 100644 --- a/integration-tests/worker/test/integration.test.ts +++ b/integration-tests/worker/test/integration.test.ts @@ -21,7 +21,10 @@ test.before(async () => { maxWorkers: 1, repoDir: path.resolve('tmp/repo/integration'), }; - const workerArgs = { runPublicKey: keys.public }; + const workerArgs = { + runPublicKey: keys.public, + monorepoDir: path.resolve('monorepo'), + }; ({ worker, engine, engineLogger } = await initWorker( lightningPort, @@ -599,6 +602,27 @@ test.serial('Include timestamps on basically everything', (t) => { }); }); +test.serial('use local adaptor versions from monorepo', (t) => { + return new Promise(async (done) => { + lightning.once('run:complete', (evt) => { + const result = lightning.getResult('a1'); + t.deepEqual(result, { data: 42 }); + done(); + }); + + lightning.enqueueRun({ + id: 'a1', + jobs: [ + { + id: 'j1', + body: 'fortyTwo()', + adaptor: '@openfn/language-common@local', + }, + ], + }); + }); +}); + test.serial("Don't send adaptor logs to stdout", (t) => { return new Promise(async (done) => { // We have to create a new worker with a different repo for this one From b5ef52726fec75c3f94480ab91c012f7cdad0ad2 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Tue, 29 Oct 2024 18:36:17 +0000 Subject: [PATCH 50/52] fix test --- packages/ws-worker/test/lightning.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/ws-worker/test/lightning.test.ts b/packages/ws-worker/test/lightning.test.ts index 8c490e45f..4614442fd 100644 --- a/packages/ws-worker/test/lightning.test.ts +++ b/packages/ws-worker/test/lightning.test.ts @@ -43,6 +43,7 @@ test.before(async () => { secret: 'abc', maxWorkflows: 1, collectionsVersion: '1.0.0', + collectionsUrl: 'www', // Note that if this is not passed, // JWT verification will be skipped runPublicKey: keys.public, @@ -272,7 +273,7 @@ test.serial('should run a run with the collections adaptor', async (t) => { }; lng.waitForResult(run.id).then((result: any) => { - t.is(result.collections_endpoint, urls.lng); + t.is(result.collections_endpoint, 'www'); t.is(typeof result.collections_token, 'string'); done(); }); From 15a87405e86573ff0c0a4e67bbc2a20c210d8cbc Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Tue, 29 Oct 2024 19:01:02 +0000 Subject: [PATCH 51/52] versions --- .changeset/slow-parrots-divide.md | 5 ----- .changeset/wise-drinks-behave.md | 5 ----- packages/engine-multi/CHANGELOG.md | 1 + packages/ws-worker/CHANGELOG.md | 1 + 4 files changed, 2 insertions(+), 10 deletions(-) delete mode 100644 .changeset/slow-parrots-divide.md delete mode 100644 .changeset/wise-drinks-behave.md diff --git a/.changeset/slow-parrots-divide.md b/.changeset/slow-parrots-divide.md deleted file mode 100644 index 5d3ea1371..000000000 --- a/.changeset/slow-parrots-divide.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@openfn/ws-worker': minor ---- - -Support @local adaptor versions (which map to the monorepo) diff --git a/.changeset/wise-drinks-behave.md b/.changeset/wise-drinks-behave.md deleted file mode 100644 index bdc402950..000000000 --- a/.changeset/wise-drinks-behave.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@openfn/engine-multi': patch ---- - -Engine: don't try to autoinstall adaptor versions with @local diff --git a/packages/engine-multi/CHANGELOG.md b/packages/engine-multi/CHANGELOG.md index c52f05c7b..c14320d5f 100644 --- a/packages/engine-multi/CHANGELOG.md +++ b/packages/engine-multi/CHANGELOG.md @@ -8,6 +8,7 @@ ### Patch Changes +- Engine: don't try to autoinstall adaptor versions with @local - Updated dependencies [3463ff9] - Updated dependencies [7a85894] - Updated dependencies [b6de2c4] diff --git a/packages/ws-worker/CHANGELOG.md b/packages/ws-worker/CHANGELOG.md index a310eb866..bcbd69ae6 100644 --- a/packages/ws-worker/CHANGELOG.md +++ b/packages/ws-worker/CHANGELOG.md @@ -6,6 +6,7 @@ - fd0e499: Support collections - bcd82e9: Accept collection version at startup (as arg or auto-looked-up from npm) +- Support @local adaptor versions (which map to the monorepo) ### Patch Changes From 390a20a11fd1393856ee7e6ab0374d68d95e057d Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Wed, 30 Oct 2024 09:43:30 +0000 Subject: [PATCH 52/52] engine: fix an issue where local adaptors don't load exports properly --- packages/engine-multi/src/api/compile.ts | 28 ++++++++++-------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/packages/engine-multi/src/api/compile.ts b/packages/engine-multi/src/api/compile.ts index 41d33af67..2aa0ecfa1 100644 --- a/packages/engine-multi/src/api/compile.ts +++ b/packages/engine-multi/src/api/compile.ts @@ -1,5 +1,5 @@ import compile, { preloadAdaptorExports, Options } from '@openfn/compiler'; -import { getModulePath } from '@openfn/runtime'; +import { getModulePath, getNameAndVersion } from '@openfn/runtime'; import type { Job } from '@openfn/lexicon'; import type { Logger } from '@openfn/logger'; @@ -18,12 +18,7 @@ export default async (context: ExecutionContext) => { const job = step as Job; if (job.expression) { try { - job.expression = await compileJob( - job.expression as string, - logger, - repoDir, - job.adaptors - ); + job.expression = await compileJob(job, logger, repoDir); } catch (e) { throw new CompileError(e, job.id!); } @@ -43,12 +38,8 @@ const stripVersionSpecifier = (specifier: string) => { return specifier; }; -const compileJob = async ( - job: string, - logger: Logger, - repoDir?: string, - adaptors?: string[] -) => { +const compileJob = async (job: Job, logger: Logger, repoDir?: string) => { + const { expression, adaptors, linker } = job; const compilerOptions: Options = { logger, }; @@ -56,8 +47,13 @@ const compileJob = async ( if (adaptors && repoDir) { const adaptorConfig = []; for (const adaptor of adaptors) { - // TODO I probably don't want to log this stuff - const pathToAdaptor = await getModulePath(adaptor, repoDir, logger); + const { name } = getNameAndVersion(adaptor); + + // Support local versions by looking in job.linker for a local path to the adaptor + const pathToAdaptor = + linker && linker[name] + ? linker[name].path + : await getModulePath(adaptor, repoDir, logger); const exports = await preloadAdaptorExports(pathToAdaptor!, logger); adaptorConfig.push({ name: stripVersionSpecifier(adaptor), @@ -69,5 +65,5 @@ const compileJob = async ( adaptors: adaptorConfig, }; } - return compile(job, compilerOptions); + return compile(expression, compilerOptions); };