OpenFn / kit

The bits & pieces that make OpenFn work. (diagrammer, cli, compiler, runtime, runtime manager, logger, etc.)
10 stars 9 forks source link

CLI: allow file paths for state and data in a workflow #715

Closed SatyamMattoo closed 3 months ago

SatyamMattoo commented 3 months ago

Short Description

Allows the user to add file paths to the state and data in a workflow.

Related issue

Fixes #639

Implementation Details

I have added a function that checks if the state is a file path. If it is, the function reads the file. Additionally, if the data (or any key inside state) is a file path, it will read that file and update the key with the file's content.

For example, if we have workflow.json:

{
    "workflow": {
        "steps": [
            {
                "id": "1",
                "adaptor": "common",
                "state": "./state.json",
                "expression": "./expression.js"
            }
        ]
    }
}

state.json:

{
    "data":"./data.json"
}

data.json:

{
    "x": 1
}

It will compile the workflow as:

{
  "workflow": {
    "steps": [
      {
        "id": "1",
        "adaptor": "@openfn/language-common",
        "state": {
          "data": {
            "x": 1
          }
        },
        "expression": "import { fn } from \"@openfn/language-common\";\nexport * from \"@openfn/language-common\";\nexport default [fn((state) => state)];"
      }
    ],
    "name": "input"
  },
  "options": {}
}

QA Notes

Added a test to verify that the file paths are identified, and the file content is successfully copied to the required key. Also tested it locally for different cases.

Checklist before requesting a review

josephjclark commented 3 months ago

Thank you @SatyamMattoo - I'll take a look at this next week :pray:

josephjclark commented 3 months ago

This will also need testing and maybe implementing in the deploy command @SatyamMattoo, which is probably the hard bit

SatyamMattoo commented 3 months ago

Okay sure, I will try doing that as well and push the changes for the review. For the testing part do you mean the integration tests or adding more tests here?

josephjclark commented 3 months ago

Actually @SatyamMattoo forget about the deploy thing - I think that's actually a separate concern. I'll merge this and raise another issue for deploy if I need to (but on reflection we may not)