Skip to content

Commit

Permalink
Merge branch 'release/1-0' of github.com-josephjclark:OpenFn/kit into…
Browse files Browse the repository at this point in the history
… release/1-0
  • Loading branch information
josephjclark committed Feb 13, 2024
2 parents 858d07e + 101f5a1 commit 3e49146
Show file tree
Hide file tree
Showing 18 changed files with 221 additions and 89 deletions.
5 changes: 5 additions & 0 deletions .changeset/sixty-snails-lie.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@openfn/cli': major
---

Autoinstall adaptors by default (pass `--no-autoinstall` to disable)
5 changes: 5 additions & 0 deletions .changeset/three-shrimps-approve.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@openfn/ws-worker': patch
---

Better error handling for invalid dataclips
4 changes: 2 additions & 2 deletions packages/cli/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,12 @@ You're probably here to run Workflows (or individual jobs), which the CLI makes

```
openfn path/to/workflow.json
openfn path/to/job.js -ia adaptor-name
openfn path/to/job.js -a adaptor-name
```

If running a single job, you MUST specify which adaptor to use.

Pass the `-i` flag to auto-install any required adaptors (it's safe to do this redundantly, although the run will be a little slower).
If the requested adaptor (or a matching version) is not already installed, it will be installed automatically. To disable this behaviour, pass the `--no-autoinstall` flag.

When finished, the CLI will write the resulting state to disk. By default the CLI will create an `output.json` next to the job file. You can pass a path to output by passing `-o path/to/output.json` and state by adding `-s path/to/state.json`. You can use `-S` and `-O` to pass state through stdin and return the output through stdout.

Expand Down
3 changes: 2 additions & 1 deletion packages/cli/src/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ const handlers = {
['repo-install']: install,
['repo-pwd']: pwd,
['repo-list']: list,
version: async (opts: Opts, logger: Logger) => printVersions(logger, opts),
version: async (opts: Opts, logger: Logger) =>
printVersions(logger, opts, true),
};

// Top level command parser
Expand Down
4 changes: 2 additions & 2 deletions packages/cli/src/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,8 @@ export const autoinstall: CLIOption = {
yargs: {
alias: ['i'],
boolean: true,
description: 'Auto-install the language adaptor',
default: false,
description: 'Auto-install the language adaptor(s)',
default: true,
},
};

Expand Down
29 changes: 19 additions & 10 deletions packages/cli/src/util/print-versions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ const loadVersionFromPath = (adaptorPath: string) => {

const printVersions = async (
logger: Logger,
options: Partial<Pick<Opts, 'adaptors' | 'logJson'>> = {}
options: Partial<Pick<Opts, 'adaptors' | 'logJson' | 'monorepoPath'>> = {},
includeComponents = false
) => {
const { adaptors, logJson } = options;
let adaptor = '';
Expand All @@ -41,6 +42,9 @@ const printVersions = async (
const [namePart, pathPart] = adaptor.split('=');
adaptorVersion = loadVersionFromPath(pathPart);
adaptorName = getNameAndVersion(namePart).name;
} else if (options.monorepoPath) {
adaptorName = getNameAndVersion(adaptor).name;
adaptorVersion = 'monorepo';
} else {
const { name, version } = getNameAndVersion(adaptor);
adaptorName = name;
Expand Down Expand Up @@ -73,23 +77,28 @@ const printVersions = async (
versions: {
'node.js': process.version.substring(1),
cli: version,
runtime: runtimeVersion,
compiler: compilerVersion,
},
};
if (includeComponents) {
output.versions.runtime = runtimeVersion;
output.versions.compiler = compilerVersion;
}
if (adaptorName) {
output.versions[adaptorName] = adaptorVersion;
}
} else {
const adaptorVersionString = adaptorName
? `\n${prefix(adaptorName)}${adaptorVersion}`
: '';

output = `Versions:
${prefix(NODE)}${process.version.substring(1)}
${prefix(CLI)}${version}
${prefix(RUNTIME)}${runtimeVersion}
${prefix(COMPILER)}${compilerVersion}${adaptorVersionString}`;
${prefix(CLI)}${version}`;

if (includeComponents) {
output += `\n${prefix(RUNTIME)}${runtimeVersion}
${prefix(COMPILER)}${compilerVersion}`;
}

if (adaptorName) {
output += `\n${prefix(adaptorName)}${adaptorVersion}`;
}
}
logger.always(output);
};
Expand Down
12 changes: 8 additions & 4 deletions packages/cli/test/commands.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ test.serial(
const state = JSON.stringify({ data: { count: 11 } });
const job = 'export default [byTwo]';
const result = await run(
`openfn --no-expand-adaptors -S ${state} -a times-two`,
`openfn --no-expand-adaptors -S ${state} -a times-two --no-autoinstall`,
job,
{
repoDir: '/repo',
Expand Down Expand Up @@ -512,9 +512,13 @@ test.serial(
async (t) => {
const job =
'fn((state) => { /* function isn\t actually called by the mock adaptor */ throw new Error("fake adaptor") });';
const result = await run('openfn -a @openfn/language-postgres', job, {
repoDir: '/repo',
});
const result = await run(
'openfn -a @openfn/language-postgres --no-autoinstall',
job,
{
repoDir: '/repo',
}
);
t.assert(result === 'execute called!');
}
);
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/test/execute/options.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ test('correct default options', (t) => {
const options = parse('execute job.js');

t.deepEqual(options.adaptors, []);
t.is(options.autoinstall, false);
t.is(options.autoinstall, true);
t.is(options.command, 'execute');
t.is(options.compile, true);
t.is(options.expandAdaptors, true);
Expand Down
73 changes: 60 additions & 13 deletions packages/cli/test/util/print-versions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import printVersions from '../../src/util/print-versions';

const root = path.resolve('package.json');

test('print versions for node, cli, runtime and compiler', async (t) => {
test('print versions for node and cli', async (t) => {
const logger = createMockLogger('', { level: 'info' });
await printVersions(logger);

Expand All @@ -17,12 +17,13 @@ test('print versions for node, cli, runtime and compiler', async (t) => {
// very crude testing but it's ok to test the intent here
t.regex(message, /Versions:/);
t.regex(message, /cli/);
t.regex(message, /runtime/);
t.regex(message, /compiler/);
t.regex(message, /node/);
t.notRegex(message, /adaptor/);
t.notRegex(message, /compiler/);
t.notRegex(message, /runtime/);
});

test('print versions for node, cli, runtime, compiler and adaptor', async (t) => {
test('print versions for node, cli and adaptor', async (t) => {
const logger = createMockLogger('', { level: 'info' });
await printVersions(logger, { adaptors: ['http'] });

Expand All @@ -31,27 +32,39 @@ test('print versions for node, cli, runtime, compiler and adaptor', async (t) =>

t.regex(message, /Versions:/);
t.regex(message, /cli/);
t.regex(message, /node/);
t.regex(message, /http .+ latest/);
});

test('print versions for node, cli, components and adaptor', async (t) => {
const logger = createMockLogger('', { level: 'info' });
await printVersions(logger, { adaptors: ['http'] }, true);

const last = logger._parse(logger._last);
const message = last.message as string;

t.regex(message, /Versions:/);
t.regex(message, /cli/);
t.regex(message, /node/);
t.regex(message, /runtime/);
t.regex(message, /compiler/);
t.regex(message, /node/);
t.regex(message, /http .+ latest/);
});

test('print versions for node, cli, runtime, compiler and adaptor with version', async (t) => {
test('print versions for node, cli and adaptor with version', async (t) => {
const logger = createMockLogger('', { level: 'info' });
await printVersions(logger, { adaptors: ['http@1234'] });

const last = logger._parse(logger._last);
const message = last.message as string;

// very crude testing but it's ok to test the intent here
t.regex(message, /Versions:/);
t.regex(message, /cli/);
t.regex(message, /runtime/);
t.regex(message, /compiler/);
t.regex(message, /node/);
t.regex(message, /http .+ 1234/);
});

test('print versions for node, cli, runtime, compiler and long-form adaptor', async (t) => {
test('print versions for node, cli and long-form adaptor', async (t) => {
const logger = createMockLogger('', { level: 'info' });
await printVersions(logger, { adaptors: ['@openfn/language-http'] });

Expand All @@ -61,7 +74,7 @@ test('print versions for node, cli, runtime, compiler and long-form adaptor', as
t.regex(message, /@openfn\/language-http .+ latest/);
});

test('print versions for node, cli, runtime, compiler and long-form adaptor with version', async (t) => {
test('print versions for node, cli and long-form adaptor with version', async (t) => {
const logger = createMockLogger('', { level: 'info' });
await printVersions(logger, { adaptors: ['@openfn/language-http@1234'] });

Expand All @@ -71,6 +84,24 @@ test('print versions for node, cli, runtime, compiler and long-form adaptor with
t.regex(message, /@openfn\/language-http .+ 1234/);
});

test('print version of adaptor with monorepo', async (t) => {
mock({
'/repo/http/package.json': '{ "version": "1.0.0" }',
[root]: mock.load(root, {}),
});

const logger = createMockLogger('', { level: 'info' });
await printVersions(logger, {
adaptors: ['@openfn/[email protected]'],
monorepoPath: '.',
});

const last = logger._parse(logger._last);
const message = last.message as string;

t.regex(message, /@openfn\/language-http(.+)monorepo/);
});

test('print version of adaptor with path', async (t) => {
mock({
'/repo/http/package.json': '{ "version": "1.0.0" }',
Expand All @@ -88,6 +119,24 @@ test('print version of adaptor with path', async (t) => {
t.regex(message, /@openfn\/language-http(.+)1\.0\.0/);
});

test('print version of adaptor with path even if monorepo is set', async (t) => {
mock({
'/repo/http/package.json': '{ "version": "1.0.0" }',
[root]: mock.load(root, {}),
});

const logger = createMockLogger('', { level: 'info' });
await printVersions(logger, {
adaptors: ['@openfn/language-http=/repo/http'],
monorepoPath: '.',
});

const last = logger._parse(logger._last);
const message = last.message as string;

t.regex(message, /@openfn\/language-http(.+)1\.0\.0/);
});

test('print version of adaptor with path and @', async (t) => {
mock({
'/repo/node_modules/@openfn/http/package.json': '{ "version": "1.0.0" }',
Expand Down Expand Up @@ -115,7 +164,5 @@ test('json output', async (t) => {
const [{ versions }] = last.message;
t.truthy(versions['node.js']);
t.truthy(versions['cli']);
t.truthy(versions['runtime']);
t.truthy(versions['compiler']);
t.truthy(versions['http']);
});
2 changes: 2 additions & 0 deletions packages/lexicon/lightning.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ export type DataClip = Record<string, any>;

export type Credential = Record<string, any>;

// TODO export reason strings from this repo
// and explain what each reason means
export type ExitReasonStrings =
| 'success'
| 'fail'
Expand Down
20 changes: 13 additions & 7 deletions packages/lightning-mock/src/api-sockets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -263,8 +263,6 @@ const createSocketAPI = (
});
}

// TODO this mock function is broken in the phoenix package update
// (I am not TOO worried, the actual integration works fine)
function getDataclip(
state: ServerState,
ws: DevSocket,
Expand All @@ -273,11 +271,19 @@ const createSocketAPI = (
const { ref, topic, join_ref } = evt;
const dataclip = state.dataclips[evt.payload.id];

// Send the data as an ArrayBuffer (our stringify function will do this)
const payload = {
status: 'ok',
response: enc.encode(stringify(dataclip)),
};
let payload;
if (dataclip) {
payload = {
status: 'ok',
response: enc.encode(stringify(dataclip)),
};
} else {
// TODO I think this actually tidier than what lightning does...
payload = {
status: 'error',
response: 'not_found',
};
}

ws.reply<GetDataClipReply>({
ref,
Expand Down
17 changes: 17 additions & 0 deletions packages/lightning-mock/test/channels/run.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,23 @@ test.serial('get dataclip through the run channel', async (t) => {
});
});

test.serial(
'get dataclip should throw if the dataclip does not exist',
async (t) => {
return new Promise(async (done) => {
server.startRun(run1.id);

const channel = await join(`run:${run1.id}`, { token: 'a.b.c' });
channel
.push(GET_DATACLIP, { id: 'x' })
.receive('error', (result: any) => {
t.is(result, 'not_found');
done();
});
});
}
);

// TODO test that all events are proxied out to server.on

test.serial(
Expand Down
Loading

0 comments on commit 3e49146

Please sign in to comment.