Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/breezy-banks-tell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@openfn/ws-worker': patch
---

Remove special edge condition mapping (this is now handled by the runtime)
7 changes: 7 additions & 0 deletions .changeset/great-facts-wish.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@openfn/runtime': minor
---

Support special condition strings `never`, `always`, `on_job_success` and `on_job_fail`.

These used to be mapped from Lightning workflows by the Worker, but by supporting them in the runtime directly we get much better compatibility across platforms
5 changes: 5 additions & 0 deletions .changeset/loud-numbers-sell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@openfn/project': patch
---

Map edge conditions so that they are compatible with CLI
5 changes: 5 additions & 0 deletions .changeset/three-lamps-double.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@openfn/cli': minor
---

Fix edge conditions in pulled workflows
2 changes: 1 addition & 1 deletion integration-tests/cli/test/project-v1.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ steps:
next:
transform-data:
disabled: false
condition: true
condition: always
- id: transform-data
name: Transform data
adaptor: "@openfn/language-common@latest"
Expand Down
3 changes: 2 additions & 1 deletion integration-tests/worker/src/factories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export const createTrigger = () => ({
id: crypto.randomUUID(),
});

export const createEdge = (a: any, b: any, condition?: string) => {
export const createEdge = (a: any, b: any, extra?: any) => {
const edge: any = {
id: crypto.randomUUID(),
target_job_id: b.id,
Expand All @@ -29,6 +29,7 @@ export const createEdge = (a: any, b: any, condition?: string) => {
} else {
edge.source_job_id = a.id;
}
Object.assign(edge, extra);
return edge;
};

Expand Down
21 changes: 21 additions & 0 deletions integration-tests/worker/test/runs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -283,3 +283,24 @@ test.serial('Run with collections', async (t) => {
{ key: 'c', value: { id: 'c' } },
]);
});

test.serial('Run with edge conditions', async (t) => {
const job1 = createJob({
body: `fn((s) => s)`,
});
const job2 = createJob({
body: `fn((s) => {
s.didExecuteStep2 = true
return s;
})`,
});
const edge = createEdge(job1, job2, {
// I would prefer this ran on failure, but that's a lot
// harder to do the way these tests are set up
condition: 'on_job_success',
});
const attempt = createRun([], [job1, job2], [edge]);

const state = await run(t, attempt);
t.true(state.didExecuteStep2);
});
2 changes: 1 addition & 1 deletion packages/cli/test/projects/checkout.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ workspace:
next: {
'transform-data-to-fhir-standard': {
disabled: false,
condition: true,
condition: 'always',
},
},
},
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/test/projects/fetch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ test.serial(
next: {
'transform-data': {
disabled: false,
condition: true,
condition: 'always',
openfn: {
uuid: 'a9a3adef-b394-4405-814d-3ac4323f4b4b',
},
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/test/projects/fixtures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ workflows:
next:
transform-data:
disabled: false
condition: true
condition: always
openfn:
uuid: a9a3adef-b394-4405-814d-3ac4323f4b4b
history:
Expand Down
28 changes: 17 additions & 11 deletions packages/project/src/parse/from-app-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,22 +62,28 @@ export default (
return new Project(proj as l.Project, config);
};

const mapTriggerEdgeCondition = (edge: Provisioner.Edge) => {
// TODO maybe this is a util and moved out of this file
export const mapEdge = (edge: Provisioner.Edge) => {
const e: any = {
disabled: !edge.enabled,
};
if (edge.condition_type === 'always') {
e.condition = true;
} else if (edge.condition_type === 'never') {
e.condition = false;
} else {

if (edge.condition_type === 'js_expression') {
e.condition = edge.condition_expression;
} else if (edge.condition_type) {
e.condition = edge.condition_type;
}

if (edge.condition_label) {
e.name = edge.condition_label;
}

// Do this last so that it serializes last
e.openfn = {
uuid: edge.id,
};
if (edge.id) {
e.openfn = {
uuid: edge.id,
};
}
return e;
};

Expand Down Expand Up @@ -114,7 +120,7 @@ export const mapWorkflow = (workflow: Provisioner.Workflow) => {
throw new Error(`Failed to find ${edge.target_job_id}`);
}
// we use the name, not the id, to reference
obj[slugify(target.name)] = mapTriggerEdgeCondition(edge);
obj[slugify(target.name)] = mapEdge(edge);
return obj;
}, {}),
} as l.Trigger);
Expand Down Expand Up @@ -148,7 +154,7 @@ export const mapWorkflow = (workflow: Provisioner.Workflow) => {
s.next = outboundEdges.reduce((next, edge) => {
const target = jobs.find((j) => j.id === edge.target_job_id);
// @ts-ignore
next[slugify(target.name)] = mapTriggerEdgeCondition(edge);
next[slugify(target.name)] = mapEdge(edge);
return next;
}, {});
}
Expand Down
19 changes: 13 additions & 6 deletions packages/project/src/serialize/to-app-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,12 +141,19 @@ const mapWorkflow = (workflow: Workflow) => {
e.source_job_id = node.id;
}

if (rules.condition === true) {
e.condition_type = 'always';
} else if (rules.condition === false) {
e.condition_type = 'never';
} else if (typeof rules.condition === 'string') {
// TODO conditional
if (rules.condition) {
if (typeof rules.condition === 'boolean') {
e.condition_type = rules.condition ? 'always' : 'never';
} else if (
rules.condition.match(
/^(always|never|on_job_success|on_job_failure)$/
)
) {
e.condition_type = rules.condition;
} else {
e.condition_type = 'js_expression';
e.condition_expression = rules.condition;
}
}
wfState.edges.push(e);
});
Expand Down
158 changes: 155 additions & 3 deletions packages/project/test/parse/from-app-state.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import test from 'ava';
import fromAppState, { mapWorkflow } from '../../src/parse/from-app-state';
import fromAppState, {
mapEdge,
mapWorkflow,
} from '../../src/parse/from-app-state';
import { clone, cloneDeep } from 'lodash-es';

import state, { withCreds } from '../fixtures/sample-v1-project';
Expand Down Expand Up @@ -90,7 +93,7 @@ test('should create a Project from prov state with a workflow', (t) => {
openfn: { enabled: true, uuid: '4a06289c-15aa-4662-8dc6-f0aaacd8a058' },
next: {
'transform-data': {
condition: true,
condition: 'always',
disabled: false,
openfn: {
uuid: 'a9a3adef-b394-4405-814d-3ac4323f4b4b',
Expand Down Expand Up @@ -130,7 +133,7 @@ test('mapWorkflow: map a simple trigger', (t) => {
type: 'webhook',
next: {
'transform-data': {
condition: true,
condition: 'always',
disabled: false,
openfn: {
uuid: 'a9a3adef-b394-4405-814d-3ac4323f4b4b',
Expand Down Expand Up @@ -219,6 +222,155 @@ test('mapWorkflow: map a job with projcet credentials onto job.configuration', (
});
});

test('mapEdge: map enabled state', (t) => {
let e;

e = mapEdge({} as any);
t.deepEqual(e, {
disabled: true,
});

e = mapEdge({
enabled: true,
} as any);
t.deepEqual(e, {
disabled: false,
});

e = mapEdge({
enabled: false,
} as any);
t.deepEqual(e, {
disabled: true,
});
});

test('mapEdge: map UUID', (t) => {
const e = mapEdge({
id: 'abc',
} as any);
t.deepEqual(e, {
disabled: true,
openfn: {
uuid: 'abc',
},
});
});

test('mapEdge: map label', (t) => {
const e = mapEdge({
condition_label: 'abc',
} as any);
t.deepEqual(e, {
disabled: true,
name: 'abc',
});
});

test('mapEdge: map conditions', (t) => {
let e;

// basically any condition type should just map
e = mapEdge({
condition_type: 'always',
} as any);
t.deepEqual(e, {
disabled: true,
condition: 'always',
});

e = mapEdge({
condition_type: 'on_job_success',
} as any);
t.deepEqual(e, {
disabled: true,
condition: 'on_job_success',
});

e = mapEdge({
condition_type: 'jam',
} as any);
t.deepEqual(e, {
disabled: true,
condition: 'jam',
});

// But js expression should override
e = mapEdge({
condition_type: 'js_expression',
condition_expression: 'abc',
} as any);
t.deepEqual(e, {
disabled: true,
condition: 'abc',
});
});

// TODO the workflow yaml is not a project yaml
// so this test doesn't work
// I'll need to pull the project yaml, with uuids, to get this to work
test.skip('mapWorkflow: map edge conditions', (t) => {
// TODO for yaml like this:
const yaml = `
workflows:
- name: Edge Conditions
jobs:
- Transform-data:
name: Transform data
adaptor: "@openfn/language-common@latest"
body: assert($.ok)
- sucess:
name: sucess
adaptor: "@openfn/language-common@latest"
body: log('All ok!')
- fail:
name: fail
adaptor: "@openfn/language-common@latest"
body: log('everything is terrible')
- custom:
name: custom
adaptor: "@openfn/language-common@latest"
body: |
// Check out the Job Writing Guide for help getting started:
// https://docs.openfn.org/documentation/jobs/job-writing-guide
triggers:
- webhook:
type: webhook
enabled: true
edges:
- webhook->Transform-data:
condition_type: always
enabled: true
target_job: Transform-data
source_trigger: webhook
- Transform-data->sucess:
condition_type: on_job_success
enabled: true
target_job: sucess
source_job: Transform-data
- Transform-data->fail:
condition_type: on_job_failure
enabled: true
target_job: fail
source_job: Transform-data
- Transform-data->custom:
condition_type: js_expression
enabled: true
target_job: custom
source_job: Transform-data
condition_expression: state.ok == 22

`;
const project = fromAppState(yaml, meta, {
format: 'yaml',
});
console.log(project.workflows[0].steps);
const { next } = project.workflows[0].steps[1];
console.log({ next });
// make sure that the condition_types get mapped to condition
// also make sure that custom conditions work (both ways)
});

test('should create a Project from prov state yaml', (t) => {
const yaml = `id: e16c5f09-f0cb-4ba7-a4c2-73fcb2f29d00
name: aaa
Expand Down
Loading